[U-Boot] [PATCH 1/2] powerpc/85xx: Add command to report errata workarounds

Add 'errata' command to report what errata we workaround
Signed-off-by: Kumar Gala galak@kernel.crashing.org --- arch/powerpc/cpu/mpc85xx/Makefile | 1 + arch/powerpc/cpu/mpc85xx/cmd_errata.c | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/cpu/mpc85xx/cmd_errata.c
diff --git a/arch/powerpc/cpu/mpc85xx/Makefile b/arch/powerpc/cpu/mpc85xx/Makefile index f064fee..e3746e6 100644 --- a/arch/powerpc/cpu/mpc85xx/Makefile +++ b/arch/powerpc/cpu/mpc85xx/Makefile @@ -32,6 +32,7 @@ START = start.o resetvec.o SOBJS-$(CONFIG_MP) += release.o SOBJS = $(SOBJS-y)
+COBJS-$(CONFIG_CMD_ERRATA) += cmd_errata.o COBJS-$(CONFIG_CPM2) += commproc.o
# supports ddr1 diff --git a/arch/powerpc/cpu/mpc85xx/cmd_errata.c b/arch/powerpc/cpu/mpc85xx/cmd_errata.c new file mode 100644 index 0000000..6824ebf --- /dev/null +++ b/arch/powerpc/cpu/mpc85xx/cmd_errata.c @@ -0,0 +1,35 @@ +/* + * Copyright 2010 Freescale Semiconductor, Inc. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#include <common.h> +#include <command.h> + +static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{ + return 0; +} + +U_BOOT_CMD( + errata, 1, 0, do_errata, + "Report errata workarounds", + "" +);

Signed-off-by: Kumar Gala galak@kernel.crashing.org --- arch/powerpc/cpu/mpc85xx/cmd_errata.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/cmd_errata.c b/arch/powerpc/cpu/mpc85xx/cmd_errata.c index 6824ebf..81078bd 100644 --- a/arch/powerpc/cpu/mpc85xx/cmd_errata.c +++ b/arch/powerpc/cpu/mpc85xx/cmd_errata.c @@ -22,9 +22,23 @@
#include <common.h> #include <command.h> +#include <linux/compiler.h> +#include <asm/processor.h>
static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) { + __maybe_unused u32 svr = get_svr(); + +#if defined(CONFIG_FSL_SATA_V2) && defined(CONFIG_FSL_SATA_ERRATUM_A001) + if (IS_SVR_REV(svr, 1, 0) && + ((SVR_SOC_VER(svr) == SVR_P1022) || + (SVR_SOC_VER(svr) == SVR_P1022_E) || + (SVR_SOC_VER(svr) == SVR_P1013) || + (SVR_SOC_VER(svr) == SVR_P1013_E))) { + puts("Work-around for Erratum SATA A001 enabled\n"); + } +#endif + return 0; }

On Wed, Jun 9, 2010 at 11:18 PM, Kumar Gala galak@kernel.crashing.org wrote:
static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) {
- __maybe_unused u32 svr = get_svr();
+#if defined(CONFIG_FSL_SATA_V2) && defined(CONFIG_FSL_SATA_ERRATUM_A001)
- if (IS_SVR_REV(svr, 1, 0) &&
- ((SVR_SOC_VER(svr) == SVR_P1022) ||
- (SVR_SOC_VER(svr) == SVR_P1022_E) ||
- (SVR_SOC_VER(svr) == SVR_P1013) ||
- (SVR_SOC_VER(svr) == SVR_P1013_E))) {
- puts("Work-around for Erratum SATA A001 enabled\n");
- }
+#endif
return 0; }
How are you planning on handling chip-specific errata? Do you forsee do_errata() containing code for every erratum of every chip?

On Jun 10, 2010, at 8:58 PM, Timur Tabi wrote:
On Wed, Jun 9, 2010 at 11:18 PM, Kumar Gala galak@kernel.crashing.org wrote:
static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) {
__maybe_unused u32 svr = get_svr();
+#if defined(CONFIG_FSL_SATA_V2) && defined(CONFIG_FSL_SATA_ERRATUM_A001)
if (IS_SVR_REV(svr, 1, 0) &&
((SVR_SOC_VER(svr) == SVR_P1022) ||
(SVR_SOC_VER(svr) == SVR_P1022_E) ||
(SVR_SOC_VER(svr) == SVR_P1013) ||
(SVR_SOC_VER(svr) == SVR_P1013_E))) {
puts("Work-around for Erratum SATA A001 enabled\n");
}
+#endif
return 0;
}
How are you planning on handling chip-specific errata? Do you forsee do_errata() containing code for every erratum of every chip?
Yes, we can split it up into functions as it grows.
- k

On Jun 9, 2010, at 11:18 PM, Kumar Gala wrote:
Signed-off-by: Kumar Gala galak@kernel.crashing.org
arch/powerpc/cpu/mpc85xx/cmd_errata.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
applied to 85xx
- k

Dear Kumar Gala,
In message 1276143535-22532-2-git-send-email-galak@kernel.crashing.org you wrote:
Signed-off-by: Kumar Gala galak@kernel.crashing.org
arch/powerpc/cpu/mpc85xx/cmd_errata.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/cmd_errata.c b/arch/powerpc/cpu/mpc85xx/cmd_errata.c index 6824ebf..81078bd 100644 --- a/arch/powerpc/cpu/mpc85xx/cmd_errata.c +++ b/arch/powerpc/cpu/mpc85xx/cmd_errata.c @@ -22,9 +22,23 @@
#include <common.h> #include <command.h> +#include <linux/compiler.h> +#include <asm/processor.h>
static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) {
- __maybe_unused u32 svr = get_svr();
+#if defined(CONFIG_FSL_SATA_V2) && defined(CONFIG_FSL_SATA_ERRATUM_A001)
- if (IS_SVR_REV(svr, 1, 0) &&
((SVR_SOC_VER(svr) == SVR_P1022) ||
(SVR_SOC_VER(svr) == SVR_P1022_E) ||
(SVR_SOC_VER(svr) == SVR_P1013) ||
(SVR_SOC_VER(svr) == SVR_P1013_E))) {
Please use a switch().
Best regards,
Wolfgang Denk

On Jul 14, 2010, at 2:30 PM, Wolfgang Denk wrote:
Dear Kumar Gala,
In message 1276143535-22532-2-git-send-email-galak@kernel.crashing.org you wrote:
Signed-off-by: Kumar Gala galak@kernel.crashing.org
arch/powerpc/cpu/mpc85xx/cmd_errata.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/cmd_errata.c b/arch/powerpc/cpu/mpc85xx/cmd_errata.c index 6824ebf..81078bd 100644 --- a/arch/powerpc/cpu/mpc85xx/cmd_errata.c +++ b/arch/powerpc/cpu/mpc85xx/cmd_errata.c @@ -22,9 +22,23 @@
#include <common.h> #include <command.h> +#include <linux/compiler.h> +#include <asm/processor.h>
static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) {
- __maybe_unused u32 svr = get_svr();
+#if defined(CONFIG_FSL_SATA_V2) && defined(CONFIG_FSL_SATA_ERRATUM_A001)
- if (IS_SVR_REV(svr, 1, 0) &&
((SVR_SOC_VER(svr) == SVR_P1022) ||
(SVR_SOC_VER(svr) == SVR_P1022_E) ||
(SVR_SOC_VER(svr) == SVR_P1013) ||
(SVR_SOC_VER(svr) == SVR_P1013_E))) {
Please use a switch().
Really? There is only 1 case, you want:
switch(SVR_SOC_VER(svr)) { case SVR_P1022: case SVR_P1022_E: case SVR_P1013: case SVR_P1013_E: .... break; }
- k

Dear Kumar Gala,
In message EDA07492-2746-46D4-963A-37F4CAE0B415@kernel.crashing.org you wrote:
+#if defined(CONFIG_FSL_SATA_V2) && > defined(CONFIG_FSL_SATA_ERRATUM_A001)
- if (IS_SVR_REV(svr, 1, 0) &&
((SVR_SOC_VER(svr) == SVR_P1022) ||
(SVR_SOC_VER(svr) == SVR_P1022_E) ||
(SVR_SOC_VER(svr) == SVR_P1013) ||
(SVR_SOC_VER(svr) == SVR_P1013_E))) {
Please use a switch().
Really? There is only 1 case, you want:
switch(SVR_SOC_VER(svr)) { case SVR_P1022: case SVR_P1022_E: case SVR_P1013: case SVR_P1013_E: .... break; }
Yes - don't you think it's much easier to write and to read?
Also it shows clearly that you are missing a default: case...
BTW: your indentation is wrong, and please sort the list...
Best regards,
Wolfgang Denk

Add 'errata' command to report what errata we workaround. Report workaround for erratum SATA-A001 on P1022/P1013.
Signed-off-by: Kumar Gala galak@kernel.crashing.org --- arch/powerpc/cpu/mpc85xx/Makefile | 1 + arch/powerpc/cpu/mpc85xx/cmd_errata.c | 51 +++++++++++++++++++++++++++++++++ include/configs/P1022DS.h | 1 + 3 files changed, 53 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/cpu/mpc85xx/cmd_errata.c
diff --git a/arch/powerpc/cpu/mpc85xx/Makefile b/arch/powerpc/cpu/mpc85xx/Makefile index a481326..4ee0e9a 100644 --- a/arch/powerpc/cpu/mpc85xx/Makefile +++ b/arch/powerpc/cpu/mpc85xx/Makefile @@ -32,6 +32,7 @@ START = start.o resetvec.o SOBJS-$(CONFIG_MP) += release.o SOBJS = $(SOBJS-y)
+COBJS-$(CONFIG_CMD_ERRATA) += cmd_errata.o COBJS-$(CONFIG_CPM2) += commproc.o
# supports ddr1 diff --git a/arch/powerpc/cpu/mpc85xx/cmd_errata.c b/arch/powerpc/cpu/mpc85xx/cmd_errata.c new file mode 100644 index 0000000..d7835c8 --- /dev/null +++ b/arch/powerpc/cpu/mpc85xx/cmd_errata.c @@ -0,0 +1,51 @@ +/* + * Copyright 2010 Freescale Semiconductor, Inc. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#include <common.h> +#include <command.h> +#include <linux/compiler.h> +#include <asm/processor.h> + +static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + __maybe_unused u32 svr = get_svr(); + +#if defined(CONFIG_FSL_SATA_V2) && defined(CONFIG_FSL_SATA_ERRATUM_A001) + if (IS_SVR_REV(svr, 1, 0)) { + switch (SVR_SOC_VER(svr)) { + case SVR_P1013: + case SVR_P1013_E: + case SVR_P1022: + case SVR_P1022_E: + puts("Work-around for Erratum SATA A001 enabled\n"); + } + } +#endif + + return 0; +} + +U_BOOT_CMD( + errata, 1, 0, do_errata, + "Report errata workarounds", + "" +); diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h index e444179..ce9a4d1 100644 --- a/include/configs/P1022DS.h +++ b/include/configs/P1022DS.h @@ -345,6 +345,7 @@ #define CONFIG_CMD_ELF #define CONFIG_CMD_IRQ #define CONFIG_CMD_SETEXPR +#define CONFIG_CMD_ERRATA
#ifdef CONFIG_PCI #define CONFIG_CMD_PCI

Dear Kumar Gala,
In message 1279142410-30629-1-git-send-email-galak@kernel.crashing.org you wrote:
Add 'errata' command to report what errata we workaround. Report workaround for erratum SATA-A001 on P1022/P1013.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
...
+#if defined(CONFIG_FSL_SATA_V2) && defined(CONFIG_FSL_SATA_ERRATUM_A001)
- if (IS_SVR_REV(svr, 1, 0)) {
switch (SVR_SOC_VER(svr)) {
case SVR_P1013:
case SVR_P1013_E:
case SVR_P1022:
case SVR_P1022_E:
puts("Work-around for Erratum SATA A001 enabled\n");
}
I think there should be a "default" case?
+++ b/include/configs/P1022DS.h @@ -345,6 +345,7 @@ #define CONFIG_CMD_ELF #define CONFIG_CMD_IRQ #define CONFIG_CMD_SETEXPR +#define CONFIG_CMD_ERRATA
Please sort the list.
Best regards,
Wolfgang Denk

On Jul 14, 2010, at 4:43 PM, Wolfgang Denk wrote:
Dear Kumar Gala,
In message 1279142410-30629-1-git-send-email-galak@kernel.crashing.org you wrote:
Add 'errata' command to report what errata we workaround. Report workaround for erratum SATA-A001 on P1022/P1013.
Signed-off-by: Kumar Gala galak@kernel.crashing.org
...
+#if defined(CONFIG_FSL_SATA_V2) && defined(CONFIG_FSL_SATA_ERRATUM_A001)
- if (IS_SVR_REV(svr, 1, 0)) {
switch (SVR_SOC_VER(svr)) {
case SVR_P1013:
case SVR_P1013_E:
case SVR_P1022:
case SVR_P1022_E:
puts("Work-around for Erratum SATA A001 enabled\n");
}
I think there should be a "default" case?
No, since if you dont match you dont have the errata.
+++ b/include/configs/P1022DS.h @@ -345,6 +345,7 @@ #define CONFIG_CMD_ELF #define CONFIG_CMD_IRQ #define CONFIG_CMD_SETEXPR +#define CONFIG_CMD_ERRATA
will do
- k

Add 'errata' command to report what errata we workaround. Report workaround for erratum SATA-A001 on P1022/P1013.
Also sorted the CONFIG_CMD_* list.
Signed-off-by: Kumar Gala galak@kernel.crashing.org --- * Sorted the CMD list
arch/powerpc/cpu/mpc85xx/Makefile | 1 + arch/powerpc/cpu/mpc85xx/cmd_errata.c | 51 +++++++++++++++++++++++++++++++++ include/configs/P1022DS.h | 6 ++-- 3 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 arch/powerpc/cpu/mpc85xx/cmd_errata.c
diff --git a/arch/powerpc/cpu/mpc85xx/Makefile b/arch/powerpc/cpu/mpc85xx/Makefile index a481326..4ee0e9a 100644 --- a/arch/powerpc/cpu/mpc85xx/Makefile +++ b/arch/powerpc/cpu/mpc85xx/Makefile @@ -32,6 +32,7 @@ START = start.o resetvec.o SOBJS-$(CONFIG_MP) += release.o SOBJS = $(SOBJS-y)
+COBJS-$(CONFIG_CMD_ERRATA) += cmd_errata.o COBJS-$(CONFIG_CPM2) += commproc.o
# supports ddr1 diff --git a/arch/powerpc/cpu/mpc85xx/cmd_errata.c b/arch/powerpc/cpu/mpc85xx/cmd_errata.c new file mode 100644 index 0000000..d7835c8 --- /dev/null +++ b/arch/powerpc/cpu/mpc85xx/cmd_errata.c @@ -0,0 +1,51 @@ +/* + * Copyright 2010 Freescale Semiconductor, Inc. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#include <common.h> +#include <command.h> +#include <linux/compiler.h> +#include <asm/processor.h> + +static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{ + __maybe_unused u32 svr = get_svr(); + +#if defined(CONFIG_FSL_SATA_V2) && defined(CONFIG_FSL_SATA_ERRATUM_A001) + if (IS_SVR_REV(svr, 1, 0)) { + switch (SVR_SOC_VER(svr)) { + case SVR_P1013: + case SVR_P1013_E: + case SVR_P1022: + case SVR_P1022_E: + puts("Work-around for Erratum SATA A001 enabled\n"); + } + } +#endif + + return 0; +} + +U_BOOT_CMD( + errata, 1, 0, do_errata, + "Report errata workarounds", + "" +); diff --git a/include/configs/P1022DS.h b/include/configs/P1022DS.h index e444179..b42b5d0 100644 --- a/include/configs/P1022DS.h +++ b/include/configs/P1022DS.h @@ -338,12 +338,12 @@ */ #include <config_cmd_default.h>
+#define CONFIG_CMD_ELF +#define CONFIG_CMD_ERRATA #define CONFIG_CMD_IRQ -#define CONFIG_CMD_PING #define CONFIG_CMD_I2C #define CONFIG_CMD_MII -#define CONFIG_CMD_ELF -#define CONFIG_CMD_IRQ +#define CONFIG_CMD_PING #define CONFIG_CMD_SETEXPR
#ifdef CONFIG_PCI

On Jun 9, 2010, at 11:18 PM, Kumar Gala wrote:
Add 'errata' command to report what errata we workaround
Signed-off-by: Kumar Gala galak@kernel.crashing.org
arch/powerpc/cpu/mpc85xx/Makefile | 1 + arch/powerpc/cpu/mpc85xx/cmd_errata.c | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/cpu/mpc85xx/cmd_errata.c
applied to 85xx
- k

Dear Kumar Gala,
In message 1276143535-22532-1-git-send-email-galak@kernel.crashing.org you wrote:
Add 'errata' command to report what errata we workaround
Signed-off-by: Kumar Gala galak@kernel.crashing.org
arch/powerpc/cpu/mpc85xx/Makefile | 1 + arch/powerpc/cpu/mpc85xx/cmd_errata.c | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/cpu/mpc85xx/cmd_errata.c
...
+static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{
- return 0;
+}
NAK. Please do not add dead code.
Please re-submit when you really add code here.
Best regards,
Wolfgang Denk

On Jul 14, 2010, at 2:29 PM, Wolfgang Denk wrote:
Dear Kumar Gala,
In message 1276143535-22532-1-git-send-email-galak@kernel.crashing.org you wrote:
Add 'errata' command to report what errata we workaround
Signed-off-by: Kumar Gala galak@kernel.crashing.org
arch/powerpc/cpu/mpc85xx/Makefile | 1 + arch/powerpc/cpu/mpc85xx/cmd_errata.c | 35 +++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/cpu/mpc85xx/cmd_errata.c
...
+static int do_errata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{
- return 0;
+}
NAK. Please do not add dead code.
Please re-submit when you really add code here.
Best regards,
Wolfgang Denk
Do you want me to just merge w/the patch that does add code here?
- k

Dear Kumar Gala,
In message 4203E0DB-91EC-4311-8C52-7EC48F570323@kernel.crashing.org you wrote:
NAK. Please do not add dead code.
Please re-submit when you really add code here.
Best regards,
Wolfgang Denk
Do you want me to just merge w/the patch that does add code here?
Yes, squash these two commits (sorry, I saw the secont patch only after sending the message).
BTW: it makes sense to wait a couple of days for reviews.
Best regards,
Wolfgang Denk

On Jul 14, 2010, at 4:00 PM, Wolfgang Denk wrote:
Dear Kumar Gala,
In message 4203E0DB-91EC-4311-8C52-7EC48F570323@kernel.crashing.org you wrote:
NAK. Please do not add dead code.
Please re-submit when you really add code here.
Best regards,
Wolfgang Denk
Do you want me to just merge w/the patch that does add code here?
Yes, squash these two commits (sorry, I saw the secont patch only after sending the message).
ok
BTW: it makes sense to wait a couple of days for reviews.
I posted this on Jun 10th. Seems like a month is a few days ;)
- k
participants (3)
-
Kumar Gala
-
Timur Tabi
-
Wolfgang Denk