[PATCH v7 3/5] Documentation: Add documentation for the APM X-Gene SoC EDAC DTS binding

[Loc Ho]

Hi,

>> I had read all the emails interaction. Yes I can write a single EDAC
>> driver. But I actually have multiple instances and single instance of
>> the same IP's. For example, I have 4 DDR controllers, 4 CPU domains,
>> one L3 and one SoC. If you can suggest to me how this would work with
>> devicetree, I might be able to make it works. But from HW point of
>> view (take the memory controller as an example), there is really 4
>> hardware blocks and some shared IP's block for status. It is much
>> simple representation with multiple instance of the driver.
>
> Your whole error reporting is done roughly through the same registers,
> as Arnd pointed out. Why can't you build your single driver around that
> and based on the error type being reported, multiplex inside a *single*
> interrupt handler to the proper decoding routines.

Yes... I can do this but I don't see how it will scale when some SoC
in the future may have more or less instance of the memory controllers
for example. I guess for now, I will just iteration 4 time with an
single base address or four resources in a loop. Let's agreed on the
DT layout and I will generate an new patch then. See below...

>
>> I think we should first agreed on the DT binding and let's not worry
>> about APEI. Then, whether we have one file or multiple file. Again,
>> the HW consist of:
>>
>> 1. One top level interrupt and status registers
>> 2. CSW, MCB A and MCB B for detection of any active memory controllers
>> 3. 4 individual memory controller instance
>> 4. 4 CPU domain instance with its own individual L2 registers
>> 5. Each CPU has instance own L1
>> 6. One L3 instance
>> 7. One SoC instance
>
> And all of those are separate IP blocks and they can be integrated with
> other, non APM hardware, or do those things all belong together?

The top level interrupt may be different and APM specific unless other
vendors adapt the same bit definitions. I highly doubt other vendor
will use the same bit definitions. The CSW is APM only. The MCB A, MCB
B, and memory controller are APM only. The L3, and SoC are APM specify
only. For L1 and L2, I will need to check with the CPU designer - but
likely APM specific.

> IOW,
> can we expect such IP blocks to appear in other hw platforms or are
> they all custom APM design which need to go together due to proprietary
> coherency protocol or whatever, for example?

See above.

>
>> As for as what is useful, we find that the memory controller and the
>> L1 and L2 are useful as it provides info for errors - for HW/system
>> debugging as well as margin of DDR.
>
> Sounds like all is useful.
>

Now let us discuss the DT layout if it is an single driver:

xgeneedac: xgeneedac at 7e800000 {
        compatible = "apm,xgene-edac";
        regmap-efuse = <&efuse>; /* efuse */
        reg = <0x0 0x78800000 0x0 0x100>, /* Top level interrupt
status resource */
                <0x0 0x7e200000 0x0 0x1000>, /* CSW for MCB active resource
                <0x0 0x7e700000 0x0 0x1000>, /* MCB A resource */
                <0x0 0x7e720000 0x0 0x1000>, /* MCB B resource */
                <0x0 0x7e800000 0x0 0x1000>, /* MCU 0 resource */
                <0x0 0x7e840000 0x0 0x1000>, /* MCU 1 resource */
                <0x0 0x7e880000 0x0 0x1000>, /* MCU 2 resource */
                <0x0 0x7e8c0000 0x0 0x1000>, /* MCU 3 resource */
                <0x0 0x7c000000 0x0 0x200000>, /* CPU 0 domain for L1 and L2 */
                <0x0 0x7c200000 0x0 0x200000>, /* CPU 1 domain for L1 and L2 */
                <0x0 0x7c400000 0x0 0x200000>, /* CPU 2 domain for L1 and L2 */
                <0x0 0x7c600000 0x0 0x200000>, /* CPU 3 domain for L1 and L2 */
                <0x0 0x7e600000 0x0 0x1000>, /* L3 resource */
                <0x0 0x7e930000 0x0 0x1000>, /* SoC bus resource */
                <0x0 0x7e000000 0x0 0x1000>, /* SoC device resource */
      interrupts = <0x0 0x20 0x4>,
                       <0x0 0x21 0x4>,
                       <0x0 0x27 0x4>;
};

Arnd, Rob, and Borislav, Please comment and let me know.

-Loc