On Thu, Sep 19, 2019 at 1:37 PM Lei Wang <[email protected]> wrote:
>
> This is the device tree bindings for new EDAC driver dmc520_edac.c.
>
> Signed-off-by: Lei Wang <[email protected]>
> Reviewed-by: James Morse <[email protected]>
>
> ---
> No change in v6.
> ---
> .../devicetree/bindings/edac/arm-dmc520.txt | 26 +++++++++++++++++++
> 1 file changed, 26 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/edac/arm-dmc520.txt
>
> diff --git a/Documentation/devicetree/bindings/edac/arm-dmc520.txt b/Documentation/devicetree/bindings/edac/arm-dmc520.txt
> new file mode 100644
> index 000000000000..71e7aa32971a
> --- /dev/null
> +++ 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...
> + 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?
> +
> +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 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
Hi Rob,
Kindly ping... I addressed your comments in below, and with one
question. Thanks!
-Lei
On 10/25/19 10:31 AM, Lei Wang wrote:
> 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
>