[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250311152700.GA3573545-robh@kernel.org>
Date: Tue, 11 Mar 2025 10:27:00 -0500
From: Rob Herring <robh@...nel.org>
To: Balamanikandan Gunasundar <balamanikandan.gunasundar@...rochip.com>
Cc: miquel.raynal@...tlin.com, richard@....at, vigneshr@...com,
krzk+dt@...nel.org, conor+dt@...nel.org,
nicolas.ferre@...rochip.com, alexandre.belloni@...tlin.com,
claudiu.beznea@...on.dev, krzysztof.kozlowski+dt@...aro.org,
linux-mtd@...ts.infradead.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] dt-bindings: mtd: microchip-nand: add atmel pmecc
On Tue, Mar 11, 2025 at 05:58:46PM +0530, Balamanikandan Gunasundar wrote:
> Add bindings for programmable multibit error correction code controller
> (PMECC).
>
> Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@...rochip.com>
> ---
>
> Changes in v2:
> - Rename filename to match compatible string
> - Add constraints for sam9x7
> - Droped unused dt labels
>
> .../devicetree/bindings/mtd/atmel-nand.txt | 61 -----------------
> .../bindings/mtd/microchip,pmecc.yaml | 67 +++++++++++++++++++
> 2 files changed, 67 insertions(+), 61 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mtd/microchip,pmecc.yaml
>
> diff --git a/Documentation/devicetree/bindings/mtd/atmel-nand.txt b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> index dbbc17a866f2..1934614a9298 100644
> --- a/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/atmel-nand.txt
> @@ -1,64 +1,3 @@
> -* ECC engine (PMECC) bindings:
> -
> -Required properties:
> -- compatible: should be one of the following
> - "atmel,at91sam9g45-pmecc"
> - "atmel,sama5d4-pmecc"
> - "atmel,sama5d2-pmecc"
> - "microchip,sam9x60-pmecc"
> - "microchip,sam9x7-pmecc", "atmel,at91sam9g45-pmecc"
> -- reg: should contain 2 register ranges. The first one is pointing to the PMECC
> - block, and the second one to the PMECC_ERRLOC block.
> -
> -Example:
> -
> - nfc_io: nfc-io@...00000 {
> - compatible = "atmel,sama5d3-nfc-io", "syscon";
> - reg = <0x70000000 0x8000000>;
> - };
> -
> - pmecc: ecc-engine@...fc070 {
> - compatible = "atmel,at91sam9g45-pmecc";
> - reg = <0xffffc070 0x490>,
> - <0xffffc500 0x100>;
> - };
> -
> - ebi: ebi@...00000 {
> - compatible = "atmel,sama5d3-ebi";
> - #address-cells = <2>;
> - #size-cells = <1>;
> - atmel,smc = <&hsmc>;
> - reg = <0x10000000 0x10000000
> - 0x40000000 0x30000000>;
> - ranges = <0x0 0x0 0x10000000 0x10000000
> - 0x1 0x0 0x40000000 0x10000000
> - 0x2 0x0 0x50000000 0x10000000
> - 0x3 0x0 0x60000000 0x10000000>;
> - clocks = <&mck>;
> -
> - nand_controller: nand-controller {
> - compatible = "atmel,sama5d3-nand-controller";
> - atmel,nfc-sram = <&nfc_sram>;
> - atmel,nfc-io = <&nfc_io>;
> - ecc-engine = <&pmecc>;
> - #address-cells = <2>;
> - #size-cells = <1>;
> - ranges;
> -
> - nand@3 {
> - reg = <0x3 0x0 0x800000>;
> - atmel,rb = <0>;
> -
> - /*
> - * Put generic NAND/MTD properties and
> - * subnodes here.
> - */
> - };
> - };
> - };
> -
> ------------------------------------------------------------------------
> -
> Deprecated bindings (should not be used in new device trees):
>
> Required properties:
> diff --git a/Documentation/devicetree/bindings/mtd/microchip,pmecc.yaml b/Documentation/devicetree/bindings/mtd/microchip,pmecc.yaml
> new file mode 100644
> index 000000000000..98260a691a2e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/microchip,pmecc.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/microchip,pmecc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip pmecc controller
> +
> +maintainers:
> + - Balamanikandan Gunasundar <balamanikandan.gunasundar@...rochip.com>
> +
> +description: |
Don't need '|' if no formatting to preserve.
> + Bindings for microchip Programmable Multibit Error Correction Code
> + Controller (PMECC). pmecc is a programmable BCH encoder/decoder. This
> + block is passed as the value to the "ecc-engine" property of microchip
> + nand flash controller node.
> +
> +properties:
> + compatible:
> + oneOf:
> + - enum:
> + - atmel,at91sam9g45-pmecc
> + - atmel,sama5d2-pmecc
> + - atmel,sama5d4-pmecc
> + - microchip,sam9x60-pmecc
> + - microchip,sam9x7-pmecc
Both alone and with a fallback should not be allowed. Pick one. A
fallback is only useful if there's a driver/client that only understands
the fallback and would work with the new implementation. IOW, if 'they
are the same'.
> + - items:
> + - const: microchip,sam9x7-pmecc
> + - const: atmel,at91sam9g45-pmecc
> + - items:
> + - const: microchip,sam9x60-pmecc
> + - const: atmel,at91sam9g45-pmecc
Combine the last 2 'items' to 1 as the fallback is the same for both.
> +
> + reg:
> + items:
> + - description: Base address and size of PMECC controller registers
> + - description: Base address and size of PMECC_ERRLOC controller
Drop 'Base address and size of '.
> +
> + clocks:
> + description: The clock source for pmecc controller
For a single clock, what else would it be? Drop.
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: microchip,sam9x7-pmecc
> + then:
> + properties:
> + clocks:
> + description: The clock source for pmecc controller
What's the purpose of this if/then? It doesn't do anything.
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + ecc-engine@...fc070 {
> + compatible = "microchip,sam9x7-pmecc";
> + reg = <0xffffe000 0x300>,
> + <0xffffe600 0x100>;
> + clocks = <&pmc 2 48>;
> + };
> --
> 2.34.1
>
Powered by blists - more mailing lists