Patchwork [U-Boot] powerpc/eeprom: cleanup mac command

login
register
mail settings
Submitter York Sun
Date June 30, 2011, 6:06 p.m.
Message ID <1309457195-8475-1-git-send-email-yorksun@freescale.com>
Download mbox | patch
Permalink /patch/102801/
State Superseded
Headers show

Comments

York Sun - June 30, 2011, 6:06 p.m.
Move mac command to board/freescale/common/sys_eeprom.c.
Change the help message to be more helpful. Print argument format.
Fix MAX_NUM_PORTS to comply with v1 NXID format.

Signed-off-by: York Sun <yorksun@freescale.com>
---
 board/freescale/common/sys_eeprom.c |   29 ++++++++++++++++++++-
 common/Makefile                     |    1 -
 common/cmd_mac.c                    |   49 -----------------------------------
 3 files changed, 28 insertions(+), 51 deletions(-)
 delete mode 100644 common/cmd_mac.c
Wolfgang Denk - July 28, 2011, 1:35 p.m.
Dear York Sun,

In message <1309457195-8475-1-git-send-email-yorksun@freescale.com> you wrote:
> Move mac command to board/freescale/common/sys_eeprom.c.
> Change the help message to be more helpful. Print argument format.
> Fix MAX_NUM_PORTS to comply with v1 NXID format.
> 
> Signed-off-by: York Sun <yorksun@freescale.com>

Why are you turning a generic, board and architecture independent
command into a FSL specific one?

This makes no sense to me, NAK.

>  board/freescale/common/sys_eeprom.c |   29 ++++++++++++++++++++-
>  common/Makefile                     |    1 -
>  common/cmd_mac.c                    |   49 -----------------------------------
>  3 files changed, 28 insertions(+), 51 deletions(-)
>  delete mode 100644 common/cmd_mac.c

In case you ever move / rename a file:  please supply the options so
git will detect the move / copy / rename.

Best regards,

Wolfgang Denk
York Sun - July 28, 2011, 4:09 p.m.
Wolfgang,

On Thu, 2011-07-28 at 15:35 +0200, Wolfgang Denk wrote:
> Dear York Sun,
> 
> In message <1309457195-8475-1-git-send-email-yorksun@freescale.com> you wrote:
> > Move mac command to board/freescale/common/sys_eeprom.c.
> > Change the help message to be more helpful. Print argument format.
> > Fix MAX_NUM_PORTS to comply with v1 NXID format.
> > 
> > Signed-off-by: York Sun <yorksun@freescale.com>
> 
> Why are you turning a generic, board and architecture independent
> command into a FSL specific one?
> 

I believe this "generic" command is only for FSL boards. Moving it to
right place.

> This makes no sense to me, NAK.
> 
> >  board/freescale/common/sys_eeprom.c |   29 ++++++++++++++++++++-
> >  common/Makefile                     |    1 -
> >  common/cmd_mac.c                    |   49 -----------------------------------
> >  3 files changed, 28 insertions(+), 51 deletions(-)
> >  delete mode 100644 common/cmd_mac.c
> 
> In case you ever move / rename a file:  please supply the options so
> git will detect the move / copy / rename.

The meaningful code was moved into an existing file, so it wasn't
renaming a file.

York
Wolfgang Denk - July 28, 2011, 6:39 p.m.
Dear York Sun,

In message <1311869371.29459.11.camel@oslab-l1> you wrote:
> 
> > Why are you turning a generic, board and architecture independent
> > command into a FSL specific one?
> 
> I believe this "generic" command is only for FSL boards. Moving it to
> right place.

Even if it supports only FSL boards at the moemnt, the change should
be in the opposite direction: move all the generic code out of the FSL
directories and make it publicly available so others can use this tool
as well.

> > In case you ever move / rename a file:  please supply the options so
> > git will detect the move / copy / rename.
> 
> The meaningful code was moved into an existing file, so it wasn't
> renaming a file.

That's what you think.  Did you check what git says about it?


So: still NAK.  Please do it the other way round.

Best regards,

Wolfgang Denk

Patch

diff --git a/board/freescale/common/sys_eeprom.c b/board/freescale/common/sys_eeprom.c
index d2ed036..b11899e 100644
--- a/board/freescale/common/sys_eeprom.c
+++ b/board/freescale/common/sys_eeprom.c
@@ -34,7 +34,7 @@ 
 #endif
 
 #ifdef CONFIG_SYS_I2C_EEPROM_NXID
-#define MAX_NUM_PORTS	23
+#define MAX_NUM_PORTS	31
 #define NXID_VERSION	1
 #endif
 
@@ -552,3 +552,30 @@  unsigned int get_cpu_board_revision(void)
 	return MPC85XX_CPU_BOARD_REV(be.major, be.minor);
 }
 #endif
+
+U_BOOT_CMD(
+	mac, 3, 1,  do_mac,
+	"display and program the system ID and MAC addresses in EEPROM",
+	"without argument\n"
+	"    - show content of system ID and MAC addresses\n"
+	"read\n"
+	"    - read EEPROM without showing\n"
+	"mac save\n"
+	"    - save to the EEPROM\n"
+	"mac id\n"
+	"    - program system id (fixed)\n"
+	"mac num <string>\n"
+	"    - program <string> as system serial number\n"
+	"mac errata <string>\n"
+	"    - program <string> as errata data\n"
+	"mac date <YYMMDDhhmmss>\n"
+	"    - program timestamp\n"
+	"mac ports <n>\n"
+	"    - program the number of ports\n"
+	"mac <n> <XX:XX:XX:XX:XX:XX>\n"
+#ifdef NXID_VERSION
+	"    - program the MAC address for port n [n=0...22]"
+#else
+	"    - program the MAC address for port n [n=0...7]"
+#endif
+);
diff --git a/common/Makefile b/common/Makefile
index f81cff9..4a1514a 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -110,7 +110,6 @@  COBJS-$(CONFIG_CMD_LDRINFO) += cmd_ldrinfo.o
 COBJS-$(CONFIG_CMD_LICENSE) += cmd_license.o
 COBJS-y += cmd_load.o
 COBJS-$(CONFIG_LOGBUFFER) += cmd_log.o
-COBJS-$(CONFIG_ID_EEPROM) += cmd_mac.o
 COBJS-$(CONFIG_CMD_MD5SUM) += cmd_md5sum.o
 COBJS-$(CONFIG_CMD_MEMORY) += cmd_mem.o
 COBJS-$(CONFIG_CMD_MFSL) += cmd_mfsl.o
diff --git a/common/cmd_mac.c b/common/cmd_mac.c
deleted file mode 100644
index 1884c2a..0000000
--- a/common/cmd_mac.c
+++ /dev/null
@@ -1,49 +0,0 @@ 
-/*
- * Copyright 2006 Freescale Semiconductor
- * York Sun (yorksun@freescale.com)
- *
- * 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>
-
-extern int do_mac(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
-
-U_BOOT_CMD(
-	mac, 3, 1,  do_mac,
-	"display and program the system ID and MAC addresses in EEPROM",
-	"[read|save|id|num|errata|date|ports|0|1|2|3|4|5|6|7]\n"
-	"read\n"
-	"    - show content of EEPROM\n"
-	"mac save\n"
-	"    - save to the EEPROM\n"
-	"mac id\n"
-	"    - program system id\n"
-	"mac num\n"
-	"    - program system serial number\n"
-	"mac errata\n"
-	"    - program errata data\n"
-	"mac date\n"
-	"    - program date\n"
-	"mac ports\n"
-	"    - program the number of ports\n"
-	"mac X\n"
-	"    - program the MAC address for port X [X=0...7]"
-);