2022-05-10 04:11:09

by Medad Young

[permalink] [raw]
Subject: [PATCH v9 0/3] EDAC: nuvoton: Add nuvoton NPCM memory controller driver

Support memory controller for Nuvoton NPCM SoC.

Addressed comments from:
- Rob Herring : https://lkml.org/lkml/2022/2/25/1103
- Krzysztof Kozlowski : https://lkml.org/lkml/2022/2/27/63
- Rob Herring : https://lkml.org/lkml/2022/3/2/828
- Krzysztof Kozlowski : https://lkml.org/lkml/2022/3/11/294
- Jonathan Neuschäfer : https://lkml.org/lkml/2022/3/11/1167
- Krzysztof Kozlowski : https://lkml.org/lkml/2022/3/11/293
- Rob Herring : https://lkml.org/lkml/2022/3/11/575
- Krzysztof Kozlowski : https://lkml.org/lkml/2022/3/11/305
- Avi Fishman : https://lkml.org/lkml/2022/3/13/339
- Krzysztof Kozlowski : https://lkml.org/lkml/2022/3/14/93
- Krzysztof Kozlowski : https://lkml.org/lkml/2022/3/14/95
- Krzysztof Kozlowski : https://lkml.org/lkml/2022/3/15/378
- Boris Petkov : https://lkml.org/lkml/2022/3/17/561
- Paul Menzel : https://lkml.org/lkml/2022/4/9/47
- Paul Menzel : https://lkml.org/lkml/2022/4/11/182
- Borislav Petkov : https://lkml.org/lkml/2022/4/8/871
- Paul Menzel : https://lkml.org/lkml/2022/4/9/51
- Paul Menzel : https://lkml.org/lkml/2022/4/9/65
- Rob Herring : https://lkml.org/lkml/2022/4/21/681
- Paul Menzel : https://lkml.org/lkml/2022/5/3/307
- Paul Menzel : https://lkml.org/lkml/2022/5/3/304
- Borislav Petkov : https://lkml.org/lkml/2022/5/3/343

Changes since version 9:
- Add a necessary blank line in Kconfig for EDAC_NPCM.
- Reflow for 75 characters per line in commit message of devicetree file.
- Remove wrong tags in all the commit message.
- Reorder content in commit message of NPCM memory controller driver.

Changes since version 8:
- Add new line character at the end of file of yaml file

Changes since version 7:
- Refactor npcm_edac.c.
- Sort strings in npcm_edac.c.
- Reflow code for 75 characters per line.
- Summarize errors and warnings reported by kernel test robot.
- Shorten name of values to make them become more readable in npcm_edac.c..
- Put spaces between the * and the text in npcm_edac.c.

Changes since version 6:
- Fix warnings in npcm_edac.c.
- Add information reported by kernel test robot <[email protected]>.

Changes since version 5:
- Update commit message of dt-bindings: edac: nuvoton: add NPCM memory controller.

Changes since version 4:
- Update filename in nuvoton,npcm-memory-controller.yaml.
- Add COMPILE_TEST in Kconfig.
- Fix errors in npcm_edac.c.
- Remove unnecessary checking after of_match_device() and of_device_get_match_data().

Changes since version 3:
- Rename npcm-edac.yaml as nuvoton,npcm-memory-controller.yaml.
- Drop 'EDAC' in title of nuvoton,npcm-memory-controller.yaml.
- Update compatible in nuvoton,npcm-memory-controller.yaml.

Changes since version 2:
- Update description and compatible in npcm-edac.yaml.
- Remove address-cells and size-cells in npcm-edac.yaml.
- Reorder the items of examples in npcm-edac.yaml.
- Reorder header file in driver.

Changes since version 1:
- Add nuvoton,npcm750-memory-controller property in NPCM devicetree.
- Add new property in edac binding document.
- Add new driver for nuvoton NPCM memory controller.
Medad CChien (3):
ARM: dts: nuvoton: Add memory controller node
dt-bindings: edac: nuvoton: add NPCM memory controller
EDAC: nuvoton: Add NPCM memory controller driver

.../edac/nuvoton,npcm-memory-controller.yaml | 61 ++
arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 7 +
drivers/edac/Kconfig | 10 +
drivers/edac/Makefile | 1 +
drivers/edac/npcm_edac.c | 680 ++++++++++++++++++
5 files changed, 759 insertions(+)
create mode 100644 Documentation/devicetree/bindings/edac/nuvoton,npcm-memory-controller.yaml
create mode 100644 drivers/edac/npcm_edac.c

--
2.17.1



2022-05-10 08:51:07

by Medad Young

[permalink] [raw]
Subject: [PATCH v9 1/3] ARM: dts: nuvoton: Add memory controller node

ECC must be configured in the BootBlock header.
Then, you can read error counts via the EDAC kernel framework.

Signed-off-by: Medad CChien <[email protected]>
---
arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
index 3696980a3da1..ba542b26941e 100644
--- a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
+++ b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
@@ -106,6 +106,13 @@
interrupt-parent = <&gic>;
ranges;

+ mc: memory-controller@f0824000 {
+ compatible = "nuvoton,npcm750-memory-controller";
+ reg = <0x0 0xf0824000 0x0 0x1000>;
+ interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ };
+
rstc: rstc@f0801000 {
compatible = "nuvoton,npcm750-reset";
reg = <0xf0801000 0x70>;
--
2.17.1


2022-05-10 13:48:56

by Medad Young

[permalink] [raw]
Subject: [PATCH v9 2/3] dt-bindings: edac: nuvoton: add NPCM memory controller

Document devicetree bindings for the Nuvoton BMC NPCM memory controller.

Signed-off-by: Medad CChien <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
---
.../edac/nuvoton,npcm-memory-controller.yaml | 61 +++++++++++++++++++
1 file changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/edac/nuvoton,npcm-memory-controller.yaml

diff --git a/Documentation/devicetree/bindings/edac/nuvoton,npcm-memory-controller.yaml b/Documentation/devicetree/bindings/edac/nuvoton,npcm-memory-controller.yaml
new file mode 100644
index 000000000000..6f37211796a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/edac/nuvoton,npcm-memory-controller.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/edac/nuvoton,npcm-memory-controller.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton NPCM Memory Controller
+
+maintainers:
+ - Medad CChien <[email protected]>
+
+description: |
+ The Nuvoton BMC SoC supports DDR4 memory with and without ECC (error
+ correction check).
+
+ The memory controller supports single bit error correction, double bit
+ error detection (in-line ECC in which a section (1/8th) of the memory
+ device used to store data is used for ECC storage).
+
+ Note, the bootloader must configure ECC mode for the memory controller.
+
+properties:
+ compatible:
+ enum:
+ - nuvoton,npcm750-memory-controller
+ - nuvoton,npcm845-memory-controller
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ minItems: 1
+ items:
+ - description: uncorrectable error interrupt
+ - description: correctable error interrupt
+
+ interrupt-names:
+ minItems: 1
+ items:
+ - const: ue
+ - const: ce
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ ahb {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ mc: memory-controller@f0824000 {
+ compatible = "nuvoton,npcm750-memory-controller";
+ reg = <0x0 0xf0824000 0x0 0x1000>;
+ interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
+ };
+ };
--
2.17.1


2022-05-18 17:44:44

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v9 1/3] ARM: dts: nuvoton: Add memory controller node

On Tue, May 10, 2022 at 11:10:54AM +0800, Medad CChien wrote:
> ECC must be configured in the BootBlock header.
> Then, you can read error counts via the EDAC kernel framework.
>
> Signed-off-by: Medad CChien <[email protected]>
> ---
> arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> index 3696980a3da1..ba542b26941e 100644
> --- a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> +++ b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> @@ -106,6 +106,13 @@
> interrupt-parent = <&gic>;
> ranges;
>
> + mc: memory-controller@f0824000 {
> + compatible = "nuvoton,npcm750-memory-controller";
> + reg = <0x0 0xf0824000 0x0 0x1000>;
> + interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + };
> +
> rstc: rstc@f0801000 {
> compatible = "nuvoton,npcm750-reset";
> reg = <0xf0801000 0x70>;
> --

Please integrate scripts/checkpatch.pl into your patch creation
workflow. Some of the warnings/errors *actually* make sense.

In this case:

WARNING: DT compatible string "nuvoton,npcm750-memory-controller" appears un-documented -- check ./Documentation/devicetree/bindings/
#35: FILE: arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi:110:
+ compatible = "nuvoton,npcm750-memory-controller";

For that I'm guessing patch 2 needs to go first in the series.

In any case, the first two need an ACK from devicetree folks.

WARNING: From:/Signed-off-by: email address mismatch: 'From: Medad CChien <[email protected]>' != 'Signed-off-by: Medad CChien <[email protected]>'

For this one I wasn't sure so I had to ask: I guess it kinda makes sense
to have the From: be the same as your SOB email. I.e., make sure the
right authorship and SOB is maintained even when sending from machines
with broken email setups.

And that you can fix very easily: just add in your .git/config:

[user]
name = Medad CChien
email = [email protected]

and git would use that as the author and also slap a From: at the
beginning of the patch with the correct name and email address.

HTH.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-05-19 03:11:51

by Medad Young

[permalink] [raw]
Subject: Re: [PATCH v9 1/3] ARM: dts: nuvoton: Add memory controller node

Dear Borislav,

Borislav Petkov <[email protected]> 於 2022年5月19日 週四 上午1:44寫道:
>
> On Tue, May 10, 2022 at 11:10:54AM +0800, Medad CChien wrote:
> > ECC must be configured in the BootBlock header.
> > Then, you can read error counts via the EDAC kernel framework.
> >
> > Signed-off-by: Medad CChien <[email protected]>
> > ---
> > arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> > index 3696980a3da1..ba542b26941e 100644
> > --- a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> > +++ b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> > @@ -106,6 +106,13 @@
> > interrupt-parent = <&gic>;
> > ranges;
> >
> > + mc: memory-controller@f0824000 {
> > + compatible = "nuvoton,npcm750-memory-controller";
> > + reg = <0x0 0xf0824000 0x0 0x1000>;
> > + interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> > + status = "disabled";
> > + };
> > +
> > rstc: rstc@f0801000 {
> > compatible = "nuvoton,npcm750-reset";
> > reg = <0xf0801000 0x70>;
> > --
>
> Please integrate scripts/checkpatch.pl into your patch creation
> workflow. Some of the warnings/errors *actually* make sense.
>
> In this case:
>
> WARNING: DT compatible string "nuvoton,npcm750-memory-controller" appears un-documented -- check ./Documentation/devicetree/bindings/
> #35: FILE: arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi:110:
> + compatible = "nuvoton,npcm750-memory-controller";
>
> For that I'm guessing patch 2 needs to go first in the series.
>
> In any case, the first two need an ACK from devicetree folks.
>
> WARNING: From:/Signed-off-by: email address mismatch: 'From: Medad CChien <[email protected]>' != 'Signed-off-by: Medad CChien <[email protected]>'
>
> For this one I wasn't sure so I had to ask: I guess it kinda makes sense
> to have the From: be the same as your SOB email. I.e., make sure the
> right authorship and SOB is maintained even when sending from machines
> with broken email setups.
>
> And that you can fix very easily: just add in your .git/config:
>
> [user]
> name = Medad CChien
> email = [email protected]
>
> and git would use that as the author and also slap a From: at the
> beginning of the patch with the correct name and email address.
>

OK, I got it.

> HTH.
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

2022-05-19 15:37:13

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v9 1/3] ARM: dts: nuvoton: Add memory controller node

On Thu, May 19, 2022 at 08:55:53AM +0800, Medad Young wrote:
> OK, I got it.

Are you sure you did get it?

$ ./scripts/checkpatch.pl /tmp/medad-v10-1-3.patch
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29:
new file mode 100644

WARNING: From:/Signed-off-by: email address mismatch: 'From: Medad CChien <[email protected]>' != 'Signed-off-by: Medad CChien <[email protected]>'

total: 0 errors, 2 warnings, 62 lines checked

Before sending, you should really run checkpatch on your patches.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-05-21 00:18:26

by Medad Young

[permalink] [raw]
Subject: Re: [PATCH v9 1/3] ARM: dts: nuvoton: Add memory controller node

Dear Borislav,

Borislav Petkov <[email protected]> 於 2022年5月19日 週四 下午5:34寫道:
>
> On Thu, May 19, 2022 at 08:55:53AM +0800, Medad Young wrote:
> > OK, I got it.
>
> Are you sure you did get it?
>

for the second warning, I did upadate my .git/config according to your advise.
but I thought I met orthe problem, I will try to fix it

for the first warning, did I really need to fix it?

> $ ./scripts/checkpatch.pl /tmp/medad-v10-1-3.patch
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #29:
> new file mode 100644
>
> WARNING: From:/Signed-off-by: email address mismatch: 'From: Medad CChien <[email protected]>' != 'Signed-off-by: Medad CChien <[email protected]>'
>
> total: 0 errors, 2 warnings, 62 lines checked
>
> Before sending, you should really run checkpatch on your patches.
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

thanks
B.R.
Medad

2022-05-23 07:54:37

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v9 1/3] ARM: dts: nuvoton: Add memory controller node

On Fri, May 20, 2022 at 10:31:05AM +0800, Medad Young wrote:
> for the second warning, I did upadate my .git/config according to your
> advise. but I thought I met orthe problem, I will try to fix it

You need to do "git commit --amend" on the patch so that it updates the
author.

> for the first warning, did I really need to fix it?

Yes, you need to fix both.

Again, before you send, run checkpatch on your patches, one by one.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-05-23 09:07:51

by Medad Young

[permalink] [raw]
Subject: Re: [PATCH v9 1/3] ARM: dts: nuvoton: Add memory controller node

Dear Borislav,

thanks for your comments.

Borislav Petkov <[email protected]> 於 2022年5月20日 週五 下午8:09寫道:
>
> On Fri, May 20, 2022 at 10:31:05AM +0800, Medad Young wrote:
> > for the second warning, I did upadate my .git/config according to your
> > advise. but I thought I met orthe problem, I will try to fix it
>
> You need to do "git commit --amend" on the patch so that it updates the
> author.

I did do "git commit --amend",
I beleve the issue is about the mail server I used,
I use gmail to send the mail due to the mail server of my company
does't support smtp
so now I should sign the commit with my gmail account.

>
> > for the first warning, did I really need to fix it?
>
> Yes, you need to fix both.
>
> Again, before you send, run checkpatch on your patches, one by one.

OK

>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

B.R.
Medad

2022-05-23 09:44:37

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v9 1/3] ARM: dts: nuvoton: Add memory controller node

On Mon, May 23, 2022 at 05:06:07PM +0800, Medad Young wrote:
> I did do "git commit --amend",
> I beleve the issue is about the mail server I used,
> I use gmail to send the mail due to the mail server of my company
> does't support smtp
> so now I should sign the commit with my gmail account.

No, you should supply --author too - I had forgotten about that.

commit 0876b99e4aa2bf7113070c9c0f5d0ade7ad91697 (HEAD -> refs/heads/test)
Author: Medad CChien <[email protected]>
Date: Tue May 10 11:10:54 2022 +0800

ARM: dts: nuvoton: Add memory controller node

ECC must be configured in the BootBlock header.
Then, you can read error counts via the EDAC kernel framework.

Signed-off-by: Medad CChien <[email protected]>

$ git commit --amend --author="Medad CChien <[email protected]>"
[test 5d6cd85171d1] ARM: dts: nuvoton: Add memory controller node
Author: Medad CChien <[email protected]>
Date: Tue May 10 11:10:54 2022 +0800
1 file changed, 7 insertions(+)
$ git log -p -1
commit 5d6cd85171d14e67840e672e2f96a16981243424 (HEAD -> refs/heads/test)
Author: Medad CChien <[email protected]>
Date: Tue May 10 11:10:54 2022 +0800

ARM: dts: nuvoton: Add memory controller node

ECC must be configured in the BootBlock header.
Then, you can read error counts via the EDAC kernel framework.

Signed-off-by: Medad CChien <[email protected]>

$ git format-patch -1 -o /tmp/
/tmp/0001-ARM-dts-nuvoton-Add-memory-controller-node.patch

$ head /tmp/0001-ARM-dts-nuvoton-Add-memory-controller-node.patch
From 5d6cd85171d14e67840e672e2f96a16981243424 Mon Sep 17 00:00:00 2001
From: Medad CChien <[email protected]>
^^^^^^^^^^^^^^^^^^^^^^


Don't hesitate to look at the manpages if a tool doesn't do what you
expect it to do.

HTH.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-05-24 02:42:38

by Medad Young

[permalink] [raw]
Subject: Re: [PATCH v9 1/3] ARM: dts: nuvoton: Add memory controller node

Dear Borislav,

thanks for your help.

Borislav Petkov <[email protected]> 於 2022年5月23日 週一 下午5:34寫道:
>
> On Mon, May 23, 2022 at 05:06:07PM +0800, Medad Young wrote:
> > I did do "git commit --amend",
> > I beleve the issue is about the mail server I used,
> > I use gmail to send the mail due to the mail server of my company
> > does't support smtp
> > so now I should sign the commit with my gmail account.
>
> No, you should supply --author too - I had forgotten about that.
>
> commit 0876b99e4aa2bf7113070c9c0f5d0ade7ad91697 (HEAD -> refs/heads/test)
> Author: Medad CChien <[email protected]>
> Date: Tue May 10 11:10:54 2022 +0800
>
> ARM: dts: nuvoton: Add memory controller node
>
> ECC must be configured in the BootBlock header.
> Then, you can read error counts via the EDAC kernel framework.
>
> Signed-off-by: Medad CChien <[email protected]>
>
> $ git commit --amend --author="Medad CChien <[email protected]>"
> [test 5d6cd85171d1] ARM: dts: nuvoton: Add memory controller node
> Author: Medad CChien <[email protected]>
> Date: Tue May 10 11:10:54 2022 +0800
> 1 file changed, 7 insertions(+)
> $ git log -p -1
> commit 5d6cd85171d14e67840e672e2f96a16981243424 (HEAD -> refs/heads/test)
> Author: Medad CChien <[email protected]>
> Date: Tue May 10 11:10:54 2022 +0800
>
> ARM: dts: nuvoton: Add memory controller node
>
> ECC must be configured in the BootBlock header.
> Then, you can read error counts via the EDAC kernel framework.
>
> Signed-off-by: Medad CChien <[email protected]>
>
> $ git format-patch -1 -o /tmp/
> /tmp/0001-ARM-dts-nuvoton-Add-memory-controller-node.patch
>
> $ head /tmp/0001-ARM-dts-nuvoton-Add-memory-controller-node.patch
> From 5d6cd85171d14e67840e672e2f96a16981243424 Mon Sep 17 00:00:00 2001
> From: Medad CChien <[email protected]>
> ^^^^^^^^^^^^^^^^^^^^^^
>
>
> Don't hesitate to look at the manpages if a tool doesn't do what you
> expect it to do.

OK, I will try to supply --author with my original mail server

>
> HTH.
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

B.R.
Medad