| Message ID | 20220910195659.11843-1-Sergey.Semin@baikalelectronics.ru |
|---|---|
| Headers | show |
| Series | EDAC/synopsys: Add generic resources and Baikal-T1 support | expand |
On Sat, 10 Sep 2022 22:56:48 +0300, Serge Semin wrote: > Baikal-T1 DDR controller is based on the DW uMCTL2 DDRC IP-core v2.51a > with up to DDR3 protocol capability and 32-bit data bus + 8-bit ECC. There > are individual IRQs for each ECC and DFI events. The dedicated scrubber > clock source is absent since it's fully synchronous to the core clock. > In addition to that the DFI-DDR PHY CSRs can be accessed via a separate > registers space. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Changelog v2: > - Keep the alphabetically ordered compatible strings list. (@Krzysztof) > - Fix grammar nitpicks in the patch log. (@Krzysztof) > - Drop the PHY CSR region. (@Rob) > - Move the device bindings to the separate DT-schema. > --- > .../memory-controllers/baikal,bt1-ddrc.yaml | 91 +++++++++++++++++++ > 1 file changed, 91 insertions(+) > create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-ddrc.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: ./Documentation/devicetree/bindings/memory-controllers/baikal,bt1-ddrc.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/memory-controllers/snps,dw-umctl2-ddrc.yaml Error: Documentation/devicetree/bindings/memory-controllers/baikal,bt1-ddrc.example.dts:41.30-31 syntax error FATAL ERROR: Unable to parse input tree make[1]: *** [scripts/Makefile.lib:384: Documentation/devicetree/bindings/memory-controllers/baikal,bt1-ddrc.example.dtb] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1420: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On Sat, Sep 10, 2022 at 10:56:47PM +0300, Serge Semin wrote: > At the current state the DW uMCTL2 DDRC DT-schema can't be used as the > common one for all the IP-core-based devices due to the compatible string > property constraining the list of the supported device names. In order to > fix that we suggest to update the compatible property constraints so one > would permit having any value aside with the generic device names. At the > same time the generic DT-schema selection must be restricted to the > denoted generic devices only so not to permit the generic fallback > compatibles. Finally since the generic schema will be referenced from the > vendor-specific DT-bindings with possibly non-standard properties defined > it must permit having additional properties specified. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Note alternatively we could drop the "additionalProperties" keyword > modification since currently there is no actual device available with the > properties not listed in the generic DT-schema. Normally, this has required 2 schema files. However, I think you can do something like this: if: compatible: enum: - snps,ddrc-3.80a - snps,dw-umctl2-ddrc - xlnx,zynqmp-ddrc-2.40a then: unevaluatedProperties: false But please make sure that actually catches undocumented properties because unevaluateProperties under 'then' is not something I've tried. Rob
On Sat, 10 Sep 2022 22:56:48 +0300, Serge Semin wrote: > Baikal-T1 DDR controller is based on the DW uMCTL2 DDRC IP-core v2.51a > with up to DDR3 protocol capability and 32-bit data bus + 8-bit ECC. There > are individual IRQs for each ECC and DFI events. The dedicated scrubber > clock source is absent since it's fully synchronous to the core clock. > In addition to that the DFI-DDR PHY CSRs can be accessed via a separate > registers space. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > --- > > Changelog v2: > - Keep the alphabetically ordered compatible strings list. (@Krzysztof) > - Fix grammar nitpicks in the patch log. (@Krzysztof) > - Drop the PHY CSR region. (@Rob) > - Move the device bindings to the separate DT-schema. > --- > .../memory-controllers/baikal,bt1-ddrc.yaml | 91 +++++++++++++++++++ > 1 file changed, 91 insertions(+) > create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-ddrc.yaml > Reviewed-by: Rob Herring <robh@kernel.org>
On Mon, Sep 26, 2022 at 5:56 AM Serge Semin <fancer.lancer@gmail.com> wrote: > > On Mon, Sep 12, 2022 at 09:32:19AM -0500, Rob Herring wrote: > > On Sat, Sep 10, 2022 at 10:56:47PM +0300, Serge Semin wrote: > > > At the current state the DW uMCTL2 DDRC DT-schema can't be used as the > > > common one for all the IP-core-based devices due to the compatible string > > > property constraining the list of the supported device names. In order to > > > fix that we suggest to update the compatible property constraints so one > > > would permit having any value aside with the generic device names. At the > > > same time the generic DT-schema selection must be restricted to the > > > denoted generic devices only so not to permit the generic fallback > > > compatibles. Finally since the generic schema will be referenced from the > > > vendor-specific DT-bindings with possibly non-standard properties defined > > > it must permit having additional properties specified. > > > > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> > > > > > > --- > > > > > > Note alternatively we could drop the "additionalProperties" keyword > > > modification since currently there is no actual device available with the > > > properties not listed in the generic DT-schema. > > > > > Normally, this has required 2 schema files. However, I think you can > > do something like this: > > > > if: > > compatible: > > enum: > > - snps,ddrc-3.80a > > - snps,dw-umctl2-ddrc > > - xlnx,zynqmp-ddrc-2.40a > > then: > > unevaluatedProperties: false > > > > > > But please make sure that actually catches undocumented properties > > because unevaluateProperties under 'then' is not something I've tried. > > Oh, I wish this would work! Alas it doesn't. AFAIU the schemas under > the "then" and "else" keywords are considered as separate schemas > and are independently applied to the DT node. As soon as I added the > construction suggested by you the schema evaluation started failing > with error as none of the DT-node properties in the examples are valid: > > < ... /snps,dw-umctl2-ddrc.example.dtb: memory-controller@fd070000: > < Unevaluated properties are not allowed ('compatible', 'reg', interrupts', 'interrupt-names', '$nodename' were unexpected) > > < ... /snps,dw-umctl2-ddrc.example.dtb: memory-controller@3d400000: > < Unevaluated properties are not allowed ('compatible', 'reg', 'interrupts', 'interrupt-names', 'clocks', 'clock-names', '$nodename' were unexpected) Indeed. While unevaluatedProperties takes if/then/else into account, flipping it around doesn't. > Any suggestion of how this could be fixed? Perhaps updating the > dtschema tool anyhow? (I failed to find a quick-fix for it) Creating > an additional separate schema with the common properties seems a bit > overkill in this case. On the other hand is there a decent > alternative? I don't think there is any other fix. > What about accepting what I suggested in this patch? It does permit > additional properties, but we won't need to have a separate schema > with just several common properties. No. You can't have it both ways. Either it is a common schema or a specific device schema. Rob
This patchset is a third one in the series created in the framework of my Baikal-T1 DDRC-related work: [1: In-progress] EDAC/mc/synopsys: Various fixes and cleanups Link: https://lore.kernel.org/linux-edac/20220910194237.10142-1-Sergey.Semin@baikalelectronics.ru [2: In-progress] EDAC/synopsys: Add generic DDRC info and address mapping Link: https://lore.kernel.org/linux-edac/20220910195007.11027-1-Sergey.Semin@baikalelectronics.ru [3: In-progress] EDAC/synopsys: Add generic resources and Baikal-T1 support Link: ---you are looking at it--- Note the patchsets above must be merged in the same order as they are placed in the list in order to prevent conflicts. Nothing prevents them from being reviewed synchronously though. Any tests are very welcome. Thanks in advance. This is a final patchset in the framework of my Synopsys DW uMCTL2 DDRC work, which completes the driver updates with the new functionality and at the closure introduces the Baikal-T1 DDRC support. The series starts from extending the Synopsys DW uMCTL2 DDRC DT-schema with the controller specific IRQs, clocks and resets properties. In addition to the Baikal-T1 DDRC is added to the DT-bindings since it's based on the DW uMCTL2 DDRC v2.61a. After that we suggest to finally inform the MCI core with the detected SDRAM ranks and make sure the detected errors are reported to the corresponding rank. Then we extend the DDRC capabilities with optional Scrub functionality. It's indeed possible to have the DW uMCTL2 controller with no HW-accelerated Scrub support (no RMW engine). In that case the MCI core is supposed to perform the erroneous location ECC update by means of the platform-specific scrub method. Then we get to fix the error-injection functionality a bit. First since the driver now has the Sys<->SDRAM address translation infrastructure we can use it to convert the supplied poisonous system address to the SDRAM one. Thus there is no longer need in preserving the address in the device private data. Second we suggest to add a DebuFS node-based command to disable the error-injection feature (no idea why it hasn't been done in the first place). Afterwards a series of the IRQ-related patches goes. First we introduce the individual DDRC event IRQs support in accordance with what has been added to the DT-bindings and what the native DW uMCTL2 DDR controller actually provides. Then aside to the ECC CE/UE errors detection we suggest to the DFI/SDRAM CRC/Parity errors report. It specifically useful for the DDR4 memory which has dedicated ALARM_n signal, but can be still utilized in the framework of the older protocols if the device DFI-PHY calculates the HIF-interface signals parity. Third after adding the platform clock/resets request procedure we introduce the HW-accelerated Scrubber support. Its performance can be tuned by means of the sdram_scrub_rate SysFS node and the Core clock rate. Note it is possible to one-time-run the Scrubber in the back-to-back mode so to perform a burst-like scan of the whole SDRAM memory. At the patchset closure we finally fix the DW uMCTL2 DDRC kernel config to be available not only on the Xilinx, Intel and MXC platforms and add the Baikal-T1 DDRC support which the whole work has been dedicated for in the first place. Link: https://lore.kernel.org/linux-edac/20220822191957.28546-1-Sergey.Semin@baikalelectronics.ru/ Changelog v2: - Replace "snps,ddrc-3.80a" compatible string with "snps,dw-umctl2-ddrc" in the example. - Move unrelated changes in to the dedicated patches. (@Krzysztof) - Use the IRQ macros in the example. (@Krzysztof) - Add a new patch: [PATCH v2 01/15] dt-bindings: memory: snps: Replace opencoded numbers with macros (@Krzysztof) - Add a new patch: [PATCH v2 03/15] dt-bindings: memory: snps: Convert the schema to being generic (@Krzysztof) - Drop the PHY CSR region. (@Rob) - Move the Baikal-T1 DDRC bindings to the separate DT-schema. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> Cc: Michail Ivanov <Michail.Ivanov@baikalelectronics.ru> Cc: Pavel Parkhomenko <Pavel.Parkhomenko@baikalelectronics.ru> Cc: Punnaiah Choudary Kalluri <punnaiah.choudary.kalluri@xilinx.com> Cc: Manish Narani <manish.narani@xilinx.com> Cc: Dinh Nguyen <dinguyen@kernel.org> Cc: James Morse <james.morse@arm.com> Cc: Robert Richter <rric@kernel.org> Cc: Rob Herring <robh@kernel.org> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Cc: devicetree@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-edac@vger.kernel.org Cc: linux-kernel@vger.kernel.org Serge Semin (15): dt-bindings: memory: snps: Replace opencoded numbers with macros dt-bindings: memory: snps: Extend schema with IRQs/resets/clocks props dt-bindings: memory: snps: Convert the schema to being generic dt-bindings: memory: Add Baikal-T1 DDRC DT-schema EDAC/synopsys: Add multi-ranked memory support EDAC/synopsys: Add optional ECC Scrub support EDAC/synopsys: Drop ECC poison address from private data EDAC/synopsys: Add data poisoning disable support EDAC/synopsys: Split up ECC UE/CE IRQs handler EDAC/synopsys: Add individual named ECC IRQs support EDAC/synopsys: Add DFI alert_n IRQ support EDAC/synopsys: Add reference clocks support EDAC/synopsys: Add ECC Scrubber support EDAC/synopsys: Drop vendor-specific arch dependency EDAC/synopsys: Add Baikal-T1 DDRC support .../memory-controllers/baikal,bt1-ddrc.yaml | 91 ++ .../snps,dw-umctl2-ddrc.yaml | 83 +- drivers/edac/Kconfig | 1 - drivers/edac/synopsys_edac.c | 952 ++++++++++++++---- 4 files changed, 931 insertions(+), 196 deletions(-) create mode 100644 Documentation/devicetree/bindings/memory-controllers/baikal,bt1-ddrc.yaml