All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Gerlach <matthew.gerlach@altera.com>
To: Shankari Anand <shankari.ak0208@gmail.com>, devicetree@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>
Subject: Re: [PATCH v6 3/3] dt-bindings: memory-controllers: Convert Altera SDRAM EDAC .txt to YAML
Date: Mon, 23 Jun 2025 16:24:01 -0700	[thread overview]
Message-ID: <b97b3ca8-b65e-4e68-8995-ef11bc488930@altera.com> (raw)
In-Reply-To: <20250623195741.300644-1-shankari.ak0208@gmail.com>



On 6/23/25 12:57 PM, Shankari Anand wrote:
> Convert the Altera SOCFPGA SDRAM EDAC devicetree binding from the
> .txt format to a YAML schema.
>
> Added a 'reg' property as dt_binding_check flagged its absence.
> Dropped it from required as it causes warnings in altr,socfpga-ecc-manager.
> The controller is memory-mapped; address is confirmed from Intel's manual.
>
> Also added two strings: altr,sdram-edac-a10 and altr,sdram-edac-s10,
> compatible with altr,sdram-edac but use two interrupts;
> Schema enforces interrupt count per variant.
>
> Signed-off-by: Shankari Anand <shankari.ak0208@gmail.com>
> ---
> v5 -> v6:
> - Drop dummy intc/sdr/model/compatible from examples
> - Make reg optional to fix warning with ecc-manager binding
> - Reduced the examples to one
> ---
>   .../arm/altera/socfpga-sdram-edac.txt         | 15 -----
>   .../memory-controllers/altr,sdram-edac.yaml   | 65 +++++++++++++++++++
>   2 files changed, 65 insertions(+), 15 deletions(-)
>   delete mode 100644 Documentation/devicetree/bindings/arm/altera/socfpga-sdram-edac.txt
>   create mode 100644 Documentation/devicetree/bindings/memory-controllers/altr,sdram-edac.yaml
>
> diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-sdram-edac.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-sdram-edac.txt
> deleted file mode 100644
> index f5ad0ff69fae..000000000000
> --- a/Documentation/devicetree/bindings/arm/altera/socfpga-sdram-edac.txt
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -Altera SOCFPGA SDRAM Error Detection & Correction [EDAC]
> -The EDAC accesses a range of registers in the SDRAM controller.
> -
> -Required properties:
> -- compatible : should contain "altr,sdram-edac" or "altr,sdram-edac-a10"
> -- altr,sdr-syscon : phandle of the sdr module
> -- interrupts : Should contain the SDRAM ECC IRQ in the
> -	appropriate format for the IRQ controller.
> -
> -Example:
> -	sdramedac {
> -		compatible = "altr,sdram-edac";
> -		altr,sdr-syscon = <&sdr>;
> -		interrupts = <0 39 4>;
> -	};
> diff --git a/Documentation/devicetree/bindings/memory-controllers/altr,sdram-edac.yaml b/Documentation/devicetree/bindings/memory-controllers/altr,sdram-edac.yaml
> new file mode 100644
> index 000000000000..96677fdfa7f8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/altr,sdram-edac.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-controllers/altr,sdram-edac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Altera SoCFPGA SDRAM EDAC Controller
> +
> +maintainers:
> +  - Matthew Gerlach <matthew.gerlach@altera.com>
> +
> +description: |
> +  EDAC-compatible controller for SDRAM error detection and correction on
> +  Altera (Intel) SoCFPGA platforms.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - altr,sdram-edac
> +      - altr,sdram-edac-a10
> +      - altr,sdram-edac-s10
> +
> +  reg:
> +    maxItems: 1
> +
> +  altr,sdr-syscon:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Phandle to the SDRAM system controller (SDR) syscon node.
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 2
> +
> +required:
> +  - compatible
> +  - altr,sdr-syscon
> +  - interrupts
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - altr,sdram-edac-a10
> +              - altr,sdram-edac-s10
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 2
I believe the above if/then is causing the following error for the 
stratix10 and agilex variants:
/tmp/mgerlach/git/linux-next/arch/arm64/boot/dts/altera/socfpga_stratix10_socdk.dtb: 
sdramedac: interrupts: [[16, 4]] is too short

/tmp/mgerlach/git/linux-next/arch/arm64/boot/dts/intel/socfpga_agilex_socdk.dtb: 
sdramedac: interrupts: [[16, 4]] is too short

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    memory-controller@f8004000 {
> +        compatible = "altr,sdram-edac-a10";
> +        reg = <0xf8004000 0x1000>;
> +        altr,sdr-syscon = <&sdr>;
> +        interrupt-parent = <&intc>;
> +        interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
You need a ',' instead a ';' above.
> +                     	<GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
The line above gives me the following error when I applied the patch:
Applying: dt-bindings: memory-controllers: Convert Altera SDRAM EDAC 
.txt to YAML
.git/rebase-apply/patch:104: space before tab in indent.
                          <GIC_SPI 46 IRQ_TYPE_LEVEL_HIGH>;
warning: 1 line adds whitespace errors.

Matthew Gerlach


> +    };


  reply	other threads:[~2025-06-23 23:24 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-23 19:57 [PATCH v6 3/3] dt-bindings: memory-controllers: Convert Altera SDRAM EDAC .txt to YAML Shankari Anand
2025-06-23 23:24 ` Matthew Gerlach [this message]
2025-06-24  7:31 ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b97b3ca8-b65e-4e68-8995-ef11bc488930@altera.com \
    --to=matthew.gerlach@altera.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=robh@kernel.org \
    --cc=shankari.ak0208@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.