Re: [PATCH v6 1/2] dt-bindings: edac: arm-dmc520.txt

[Lei Wang]

Thanks James/Rob/Borislav for pointing out the email list issue. My work 
email does not work good either for this exercise. Going forward I'll 
switch to my gmail account.

And Thanks Rob for reviewing! Please see below.

> > +++ b/Documentation/devicetree/bindings/edac/arm-dmc520.txt
> > @@ -0,0 +1,26 @@
> > +* ARM DMC-520 EDAC node
> > +
> > +Required properties:
> > +- compatible           : "brcm,dmc-520", "arm,dmc-520".
> > +- reg                  : Address range of the DMC-520 registers.
> > +- interrupts           : DMC-520 interrupt numbers. The example below specifies
> > +                         two interrupt lines for dram_ecc_errc_int and
> > +                         dram_ecc_errd_int.
> > +- interrupt-config     : This is an array of interrupt masks. For each of the
>
> Not a standard property, so would need a vendor prefix...

Would dmc-interrupt-config as the property name work? Thanks!

> > +                         above interrupt line, add one interrupt mask element to
> > +                         it. That is, there is a 1:1 mapping from each interrupt
> > +                         line to an interrupt mask. An interrupt mask can represent
> > +                         multiple interrupts being enabled. Refer to interrupt_control
> > +                         register in DMC-520 TRM for interrupt mapping. In the example
> > +                         below, the interrupt configuration enables dram_ecc_errc_int
> > +                         and dram_ecc_errd_int. And each interrupt is connected to
> > +                         a separate interrupt line.
>
> I've gone and read thru the TRM some. This binding doesn't seem to
> correspond to the TRM at all. There are a bunch of interrupts and a
> combined interrupt, and then there's the same set for 'overflow'
> interrupts.
>
> There's only one 'interrupt_control' reg. How do you have more that 1
> 32-bit value?

There is only one 'interrupt_control' register, for multiple interrupt 
sources. Then depending on platform hardware design, these interrupt 
sources can be wired to different physical interrupt lines.

That is, it is possible to mux interrupt sources into  interrupt lines 
for dmc520 in different ways. For example, in this particular brcm 
implementation,

Line 841: source dram_ecc_errc_int
Line 843: source dram_ecc_errd_int
Line 839: source dram_ecc_errc_int and dram_ecc_errd_int

There are two possibilities for implementing ecc counts for ce/ue. And 
we chose to use the single source line: as below, two interrupt lines 
0x349 and 0x34B, with interrupt masks 0x4 and 0x8 respectively.

Also, it's possible to implement using the combined-source line too: 
that would be one interrupt line 0x347, with interrupt mask 0xC.

This dt binding can support both by modifying the properties, without 
having to modify driver code.

>> +
>> +Example:
>> +
>> +dmc0: dmc@200000 {
>> +       compatible = "brcm,dmc-520", "arm,dmc-520";
>> +       reg = <0x200000 0x80000>;
>> +       interrupts = <0x0 0x349 0x4>, <0x0 0x34B 0x4>;
>> +       interrupt-config = <0x4>, <0x8>;
>> +};
>> --
>> 2.17.1

Thanks!

-Lei