Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Date:   Tue, 24 Oct 2023 11:29:19 -0500
From:   Rob Herring <robh@kernel.org>
To:     Simon Glass <sjg@chromium.org>
Cc:     devicetree@vger.kernel.org, linux-mtd@lists.infradead.org,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        Michael Walle <mwalle@kernel.org>,
        U-Boot Mailing List <u-boot@lists.denx.de>,
        Tom Rini <trini@konsulko.com>,
        Conor Dooley <conor+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Richard Weinberger <richard@nod.at>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 3/3] dt-bindings: mtd: binman-partitions: Add
 alignment properties
Message-ID: <20231024162919.GC3707756-robh@kernel.org>
References: <20231009220436.2164245-1-sjg@chromium.org>
 <20231009220436.2164245-3-sjg@chromium.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20231009220436.2164245-3-sjg@chromium.org>
Precedence: bulk

On Mon, Oct 09, 2023 at 04:04:15PM -0600, Simon Glass wrote:
> Add three properties for controlling alignment of partitions, aka
> 'entries' in binman.
> 
> For now there is no explicit mention of hierarchy, so a 'section' is
> just the 'binman' node.
> 
> These new properties are inputs to the packaging process, but are also
> needed if the firmware is repacked, to ensure that alignment
> constraints are not violated. Therefore they are provided as part of
> the schema.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> (no changes since v2)
> 
> Changes in v2:
> - Fix 'a' typo in commit message
> 
>  .../mtd/partitions/binman-partition.yaml      | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
> index 35a320359ec1..8e8a3b6d4d14 100644
> --- a/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
> +++ b/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
> @@ -28,6 +28,42 @@ properties:
>            - const: u-boot       # u-boot.bin from U-Boot project
>            - const: atf-bl31     # bl31.bin or bl31.elf from TF-A project
>  
> +  align:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      This sets the alignment of the entry. The entry offset is adjusted
> +      so that the entry starts on an aligned boundary within the containing
> +      section or image. For example ‘align = <16>’ means that the entry will
> +      start on a 16-byte boundary. This may mean that padding is added before

Only your example defines that alignment is in bytes.

> +      the entry. The padding is part of the containing section but is not
> +      included in the entry, meaning that an empty space may be created before
> +      the entry starts. Alignment should be a power of 2. If ‘align’ is not
> +      provided, no alignment is performed.

Would be nice to have some constraints. Unfortunately, no way to say 
'power of 2' in json-schema (we could add something possibly), so the 
only way is:

enum: [ 2, 4, 8, 16, 32, 64, ... ]

Kind of verbose if we add all 31 possibilities...

Could also do this:

minium: 2
maximum: 0x80000000
multipleOf: 2

> +
> +  align-size:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      This sets the alignment of the entry size. For example, to ensure
> +      that the size of an entry is a multiple of 64 bytes, set this to 64.
> +      While this does not affect the contents of the entry within binman
> +      itself (the padding is performed only when its parent section is
> +      assembled), the end result is that the entry ends with the padding
> +      bytes, so may grow. If ‘align-size’ is not provided, no alignment is
> +      performed.
> +
> +  align-end:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      This sets the alignment of the end of an entry with respect to the
> +      containing section. Some entries require that they end on an alignment
> +      boundary, regardless of where they start. This does not move the start
> +      of the entry, so the contents of the entry will still start at the
> +      beginning. But there may be padding at the end. While this does not
> +      affect the contents of the entry within binman itself (the padding is
> +      performed only when its parent section is assembled), the end result is
> +      that the entry ends with the padding bytes, so may grow. If ‘align-end’
> +      is not provided, no alignment is performed.
> +
>  additionalProperties: false
>  
>  examples:
> @@ -40,10 +76,13 @@ examples:
>          partition@100000 {
>              compatible = "u-boot";
>              reg = <0x100000 0xf00000>;
> +            align-size = <0x1000>;
> +            align-end = <0x10000>;
>          };
>  
>          partition@200000 {
>              compatible = "atf-bl31";
>              reg = <0x200000 0x100000>;
> +            align = <0x4000>;
>          };
>      };
> -- 
> 2.42.0.609.gbb76f46606-goog
>