On 22/08/2022 22:19, Serge Semin wrote:
> First of all the DW uMCTL2 DDRC IP-core supports the individual IRQ lines
> for each standard event: ECC Corrected Error, ECC Uncorrected Error, ECC
> Address Protection, Scrubber-Done signal, DFI Parity/CRC Error. It's
> possible that the platform engineers merge them up in the IRQ controller
> level. So let's add both configuration support to the DT-schema.
>
> Secondly each IP-core interface is supplied with a clock source like APB
> reference clock, AXI-ports clock, main DDRC core reference clock and
> Scrubber low-power clock. In addition to that each clock domain can have a
> dedicated reset signal. Let's add the properties for at least the denoted
> clock sources and the corresponding reset controls.
>
> Signed-off-by: Serge Semin <Sergey.Semin@xxxxxxxxxxxxxxxxxxxx>
> ---
> .../snps,dw-umctl2-ddrc.yaml | 65 +++++++++++++++++--
> 1 file changed, 60 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml b/Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml
> index 787d91d64eee..8db92210cfe1 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/snps,dw-umctl2-ddrc.yaml
> @@ -13,13 +13,13 @@ maintainers:
>
> description: |
> Synopsys DesignWare Enhanced uMCTL2 DDR Memory Controller is cappable of
Typo in original text: capable
> - working with DDR devices up to (LP)DDR4 protocol. It can be equipped
> + working with DDR devices upporting to (LP)DDR4 protocol. It can be equipped
Typo - supporting?
> with SEC/DEC ECC feature if DRAM data bus width is either 16-bits or
> 32-bits or 64-bits wide.
>
> - The ZynqMP DDR controller is based on the DW uMCTL2 v2.40a controller.
> - It has an optional SEC/DEC ECC support in 64-bit and 32-bit bus width
> - configurations.
> + For instance the ZynqMP DDR controller is based on the DW uMCTL2 v2.40a
> + controller. It has an optional SEC/DEC ECC support in 64-bit and 32-bit
> + bus width configurations.
These changes do not look related to your patch, so split them.
>
> properties:
> compatible:
> @@ -28,11 +28,55 @@ properties:
> - xlnx,zynqmp-ddrc-2.40a
>
> interrupts:
> - maxItems: 1
> + description:
> + DW uMCTL2 DDRC IP-core provides individual IRQ signal for each event":"
> + ECC Corrected Error, ECC Uncorrected Error, ECC Address Protection,
> + Scrubber-Done signal, DFI Parity/CRC Error. Some platforms may have the
> + signals merged before they reach the IRQ controller or have some of them
> + absent in case if the corresponding feature is unavailable/disabled.
> + minItems: 1
> + maxItems: 5
List has to be strictly ordered, so instead list and describe the
items... unless you are sure that any of these interrupt lines can be
merged into any other one?
> +
> + interrupt-names:
> + minItems: 1
> + maxItems: 5
> + oneOf:
> + - description: Common ECC CE/UE/Scrubber/DFI Errors IRQ
> + items:
> + - const: ecc
> + - description: Individual ECC CE/UE/Scrubber/DFI Errors IRQs
> + items:
> + enum: [ ecc_ce, ecc_ue, ecc_ap, ecc_sbr, dfi_e ]
>
> reg:
> maxItems: 1
>
> + clocks:
> + description:
> + A standard set of the clock sources contains CSRs bus clock, AXI-ports
> + reference clock, DDRC core clock, Scrubber standalone clock
> + (synchronous to the DDRC clock).
> + minItems: 1
> + maxItems: 4
I expect list to be strictly defined, not flexible.
> +
> + clock-names:
> + minItems: 1
> + maxItems: 4
> + items:
> + enum: [ pclk, aclk, core, sbr ]
> +
> + resets:
> + description:
> + Each clock domain can have separate reset signal.
> + minItems: 1
> + maxItems: 4
> +
> + reset-names:
> + minItems: 1
> + maxItems: 4
> + items:
> + enum: [ prst, arst, core, sbr ]
The same.
> +
> required:
> - compatible
> - reg
> @@ -48,4 +92,15 @@ examples:
> interrupt-parent = <&gic>;
> interrupts = <0 112 4>;
> };
> + - |
> + memory-controller@fd070000 {
> + compatible = "snps,ddrc-3.80a";
> + reg = <0x3d400000 0x400000>;
> +
> + interrupts = <0 147 4>, <0 148 4>, <0 149 4>, <0 150 4>;
Use proper defines.
> + interrupt-names = "ecc_ce", "ecc_ue", "ecc_sbr", "dfi_e";
> +
> + clocks = <&rcu 0>, <&rcu 5>, <&rcu 6>, <&rcu 7>;
> + clock-names = "pclk", "aclk", "core", "sbr";
> + };
> ...
Best regards,
Krzysztof
