diff mbox series

[5.2,v2,2/2] dt-binding: edac: add NPCM ECC documentation

Message ID 20190605141253.38554-2-ghung.quanta@gmail.com
State Not Applicable, archived
Headers show
Series [5.2,v2,1/2] edac: npcm: Add Nuvoton NPCM7xx EDAC driver | expand

Commit Message

George Hung June 5, 2019, 2:12 p.m. UTC
Add device tree documentation for Nuvoton BMC ECC

Signed-off-by: George Hung <ghung.quanta@gmail.com>
---
 .../bindings/edac/npcm7xx-sdram-edac.txt        | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt

Comments

Avi Fishman June 6, 2019, 7:41 a.m. UTC | #1
On Wed, Jun 5, 2019 at 5:19 PM George Hung <ghung.quanta@gmail.com> wrote:
>
> Add device tree documentation for Nuvoton BMC ECC
>
> Signed-off-by: George Hung <ghung.quanta@gmail.com>

Reviewed-by: Avi Fishman <avifishman70@gmail.com>

> ---
>  .../bindings/edac/npcm7xx-sdram-edac.txt        | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
>
> diff --git a/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt b/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> new file mode 100644
> index 000000000000..dd4dac59a5bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> @@ -0,0 +1,17 @@
> +Nuvoton NPCM7xx SoC EDAC device driver
> +
> +The Nuvoton NPCM7xx SoC supports DDR4 memory with/without ECC and the driver
> +uses the EDAC framework to implement the ECC detection and corrtection.
> +
> +Required properties:
> +- compatible:  should be "nuvoton,npcm7xx-sdram-edac"
> +- reg:         Memory controller register set should be <0xf0824000 0x1000>
> +- interrupts:  should be MC interrupt #25
> +
> +Example:
> +
> +       mc: memory-controller@f0824000 {
> +               compatible = "nuvoton,npcm7xx-sdram-edac";
> +               reg = <0xf0824000 0x1000>;
> +               interrupts = <0 25 4>;
> +       };
> --
> 2.21.0
>
James Morse June 6, 2019, 3:46 p.m. UTC | #2
Hi George,

On 05/06/2019 15:12, George Hung wrote:
> Add device tree documentation for Nuvoton BMC ECC

(Nit: The DT folk prefer patches adding bindings to come first in the series, before the
driver that uses them).


> diff --git a/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt b/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> new file mode 100644
> index 000000000000..dd4dac59a5bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> @@ -0,0 +1,17 @@
> +Nuvoton NPCM7xx SoC EDAC device driver
> +
> +The Nuvoton NPCM7xx SoC supports DDR4 memory with/without ECC and the driver
> +uses the EDAC framework to implement the ECC detection and corrtection.

The commit message in the driver says this is a Cadence memory controller, can we describe
what it is here, and give it an additional compatible?


Thanks,

James

> +Required properties:
> +- compatible:	should be "nuvoton,npcm7xx-sdram-edac"
> +- reg:		Memory controller register set should be <0xf0824000 0x1000>
> +- interrupts:	should be MC interrupt #25
> +Example:
> +
> +	mc: memory-controller@f0824000 {
> +		compatible = "nuvoton,npcm7xx-sdram-edac";
> +		reg = <0xf0824000 0x1000>;
> +		interrupts = <0 25 4>;
> +	};
>
Rob Herring (Arm) July 9, 2019, 1:40 a.m. UTC | #3
On Wed, Jun 05, 2019 at 10:12:53PM +0800, George Hung wrote:
> Add device tree documentation for Nuvoton BMC ECC
> 
> Signed-off-by: George Hung <ghung.quanta@gmail.com>
> ---
>  .../bindings/edac/npcm7xx-sdram-edac.txt        | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> 
> diff --git a/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt b/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> new file mode 100644
> index 000000000000..dd4dac59a5bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> @@ -0,0 +1,17 @@
> +Nuvoton NPCM7xx SoC EDAC device driver
> +
> +The Nuvoton NPCM7xx SoC supports DDR4 memory with/without ECC and the driver
> +uses the EDAC framework to implement the ECC detection and corrtection.
> +
> +Required properties:
> +- compatible:	should be "nuvoton,npcm7xx-sdram-edac"

Is this for the whole SDRAM controller or just ECC related registers? 
In the former case, the naming should just reflect the block name and 
not a Linux term.

> +- reg:		Memory controller register set should be <0xf0824000 0x1000>
> +- interrupts:	should be MC interrupt #25
> +
> +Example:
> +
> +	mc: memory-controller@f0824000 {
> +		compatible = "nuvoton,npcm7xx-sdram-edac";
> +		reg = <0xf0824000 0x1000>;
> +		interrupts = <0 25 4>;
> +	};
> -- 
> 2.21.0
>
George Hung (洪忠敬) July 9, 2019, 9:48 a.m. UTC | #4
Hi Rob,


> -----Original Message-----
> From: openbmc
> [mailto:openbmc-bounces+george.hung=quantatw.com@lists.ozlabs.org] On
> Behalf Of Rob Herring
> Sent: Tuesday, July 09, 2019 9:41 AM
> To: George Hung
> Cc: Mark Rutland; Linus Walleij; Tali Perry; paulmck@linux.ibm.com;
> wak@google.com; benjaminfair@google.com; openbmc@lists.ozlabs.org;
> tomer.maimon@nuvoton.com; devicetree@vger.kernel.org; Borislav Petkov;
> Avi.Fishman@nuvoton.com; Jonathan Cameron; Mauro Carvalho Chehab;
> linux-edac; Patrick Venture; Nicolas Ferre; linux-kernel; James Morse; Greg
> Kroah-Hartman; davem@davemloft.net
> Subject: Re: [PATCH 5.2 v2 2/2] dt-binding: edac: add NPCM ECC
> documentation
> 
> On Wed, Jun 05, 2019 at 10:12:53PM +0800, George Hung wrote:
> > Add device tree documentation for Nuvoton BMC ECC
> >
> > Signed-off-by: George Hung <ghung.quanta@gmail.com>
> > ---
> >  .../bindings/edac/npcm7xx-sdram-edac.txt        | 17
> +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> >
> > diff --git
> > a/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> > b/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> > new file mode 100644
> > index 000000000000..dd4dac59a5bd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> > @@ -0,0 +1,17 @@
> > +Nuvoton NPCM7xx SoC EDAC device driver
> > +
> > +The Nuvoton NPCM7xx SoC supports DDR4 memory with/without ECC and
> the
> > +driver uses the EDAC framework to implement the ECC detection and
> corrtection.
> > +
> > +Required properties:
> > +- compatible:	should be "nuvoton,npcm7xx-sdram-edac"
> 
> Is this for the whole SDRAM controller or just ECC related registers?
> In the former case, the naming should just reflect the block name and not a
> Linux term.

Sorry for confused naming, the address space is for the whole memory controller registers indeed,
but the driver only uses the ECC related registers.
Should I change the name to "nuvoton,npcm7xx-edac" ?

> 
> > +- reg:		Memory controller register set should be <0xf0824000
> 0x1000>
> > +- interrupts:	should be MC interrupt #25
> > +
> > +Example:
> > +
> > +	mc: memory-controller@f0824000 {
> > +		compatible = "nuvoton,npcm7xx-sdram-edac";
> > +		reg = <0xf0824000 0x1000>;
> > +		interrupts = <0 25 4>;
> > +	};
> > --
> > 2.21.0
> >
Rob Herring (Arm) July 9, 2019, 3:48 p.m. UTC | #5
On Tue, Jul 9, 2019 at 3:50 AM George Hung (洪忠敬)
<George.Hung@quantatw.com> wrote:
>
> Hi Rob,
>
>
> > -----Original Message-----
> > From: openbmc
> > [mailto:openbmc-bounces+george.hung=quantatw.com@lists.ozlabs.org] On
> > Behalf Of Rob Herring
> > Sent: Tuesday, July 09, 2019 9:41 AM
> > To: George Hung
> > Cc: Mark Rutland; Linus Walleij; Tali Perry; paulmck@linux.ibm.com;
> > wak@google.com; benjaminfair@google.com; openbmc@lists.ozlabs.org;
> > tomer.maimon@nuvoton.com; devicetree@vger.kernel.org; Borislav Petkov;
> > Avi.Fishman@nuvoton.com; Jonathan Cameron; Mauro Carvalho Chehab;
> > linux-edac; Patrick Venture; Nicolas Ferre; linux-kernel; James Morse; Greg
> > Kroah-Hartman; davem@davemloft.net
> > Subject: Re: [PATCH 5.2 v2 2/2] dt-binding: edac: add NPCM ECC
> > documentation
> >
> > On Wed, Jun 05, 2019 at 10:12:53PM +0800, George Hung wrote:
> > > Add device tree documentation for Nuvoton BMC ECC
> > >
> > > Signed-off-by: George Hung <ghung.quanta@gmail.com>
> > > ---
> > >  .../bindings/edac/npcm7xx-sdram-edac.txt        | 17
> > +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> > > b/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> > > new file mode 100644
> > > index 000000000000..dd4dac59a5bd
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
> > > @@ -0,0 +1,17 @@
> > > +Nuvoton NPCM7xx SoC EDAC device driver
> > > +
> > > +The Nuvoton NPCM7xx SoC supports DDR4 memory with/without ECC and
> > the
> > > +driver uses the EDAC framework to implement the ECC detection and
> > corrtection.
> > > +
> > > +Required properties:
> > > +- compatible:      should be "nuvoton,npcm7xx-sdram-edac"
> >
> > Is this for the whole SDRAM controller or just ECC related registers?
> > In the former case, the naming should just reflect the block name and not a
> > Linux term.
>
> Sorry for confused naming, the address space is for the whole memory controller registers indeed,
> but the driver only uses the ECC related registers.
> Should I change the name to "nuvoton,npcm7xx-edac" ?

No, you should drop the 'edac' part. The DT describes the h/w, not
what one driver (currently) uses.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt b/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
new file mode 100644
index 000000000000..dd4dac59a5bd
--- /dev/null
+++ b/Documentation/devicetree/bindings/edac/npcm7xx-sdram-edac.txt
@@ -0,0 +1,17 @@ 
+Nuvoton NPCM7xx SoC EDAC device driver
+
+The Nuvoton NPCM7xx SoC supports DDR4 memory with/without ECC and the driver
+uses the EDAC framework to implement the ECC detection and corrtection.
+
+Required properties:
+- compatible:	should be "nuvoton,npcm7xx-sdram-edac"
+- reg:		Memory controller register set should be <0xf0824000 0x1000>
+- interrupts:	should be MC interrupt #25
+
+Example:
+
+	mc: memory-controller@f0824000 {
+		compatible = "nuvoton,npcm7xx-sdram-edac";
+		reg = <0xf0824000 0x1000>;
+		interrupts = <0 25 4>;
+	};