Dear Wolfgang,
In message 1260955110-5656-6-git-send-email-vipin.kumar@st.com you wrote:
Signed-off-by: Vipin vipin.kumar@st.com
...
+int misc_init_r(void) +{ +#if defined(CONFIG_CMD_NET)
- uchar mac_id[6];
- if (!i2c_read_mac(mac_id))
- eth_setenv_enetaddr("ethaddr", mac_id);
+#endif
- setenv("verify", "n");
NAK.
Please do not enforce such a policy on all your users. Let them decide if they want this, or not.
...
Source modified to if (ethaddr from environment fails) read mac from i2c memory This should be ok. Right
+int do_setfreq(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +{
- void (*sram_setfreq) (unsigned int, unsigned int);
- unsigned int frequency;
- if (argc != 3) {
- printf("Usage:\n%s\n", cmdtp->usage);
- return 1;
- }
- frequency = simple_strtoul(argv[2], NULL, 0);
- if (frequency > 333) {
- printf("Frequency is limited to 333MHz\n");
- return 1;
- }
- sram_setfreq = memcpy((void *)0xD2801000, setfreq, setfreq_sz);
- if (!strcmp(argv[1], "ddr")) {
- sram_setfreq(DDR, frequency);
- printf("DDR frequency changed to %u\n", frequency);
- } else if (!strcmp(argv[1], "cpu")) {
- sram_setfreq(CPU, frequency);
- printf("CPU frequency changed to %u\n", frequency);
- } else {
- printf("Usage:\n%s\n", cmdtp->usage);
- return 1;
- }
- return 0;
+}
+U_BOOT_CMD(setfreq, 3, 1, do_setfreq,
- "change ddr/cpu frequency", "device[ddr/cpu] frequency");
+#endif
Do we need a new custom command for this? Either allow setting by environment variable only ("cpuclk" is already used by a number of systems), or use a common command name ("chip_config" was chosen for this after some discussion a while ago).
Code modified to use chip_config. this command is implemented in board specific file. I guess that should be ok since there is no standard interface for the command
Also, is no further checking needed? Can I set arbitrary frequencies, like 307 for CPU and 277 for DDR?
Yes, all values below 333 are acceptable. This command is added for testing purposes only so no further checking is required
- if ((buf[0] == 0x55) && (buf[1] == 0xAA)) {
- /* Valid MAC address is saved in I2C EEPROM,
- read the MAC address from the
- EEPROM & update the buffer */
Incorrect multiline comment once more.
Corrected multiline comments globally
- buf[0] = 0x55;
- buf[1] = 0xAA;
- i2c_write(0x50, /* Chip address */
- 0x0, /* Offset */
- 1, /* Address length */
- buf, /* buffer */
- 2); /* Length */
Argh. Please do not do this.
Ok, modified to single line function call
- buf[0] = 0x44;
- buf[1] = 0x66;
- i2c_read(0x50, /* Chip address */
- 0x0, /* Offset */
- 1, /* Address length */
- buf, /* buffer */
- 2); /* Length */
Ok, modified to single line function call
diff --git a/common/cmd_bdinfo.c b/common/cmd_bdinfo.c index 11c1547..412cad6 100644 --- a/common/cmd_bdinfo.c +++ b/common/cmd_bdinfo.c @@ -27,6 +27,10 @@ #include <common.h> #include <command.h>
+#if defined(CONFIG_SPEAR600) || defined(CONFIG_SPEAR3XX) +#include <asm/arch/spr_xloader_table.h> +#endif
DECLARE_GLOBAL_DATA_PTR;
static void print_num(const char *, ulong); @@ -339,6 +343,22 @@ int do_bdinfo ( cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) #endif printf ("baudrate = %d bps\n", bd->bi_baudrate);
+#if defined(CONFIG_SPEAR600) || defined(CONFIG_SPEAR3XX)
- if (bd->dramfreq == -1)
- printf("DDR Freq = Not Known\n");
- else
- printf("DDR Freq = %d\n", bd->dramfreq);
- if (bd->dramtype == DDRMOBILE)
- printf("DDR Type = MOBILE\n");
- else if (bd->dramtype == DDR2)
- printf("DDR Type = DDR2\n");
- else
- printf("DDR Type = Not Known\n");
- printf("Xloader Rev = %s\n", bd->version);
+#endif
return 0; }
I don't like such board-specific code in this common file.
Ok. But then bdinfo should ideally contain a generic callback to be implemented by board specific code.
diff --git a/include/configs/spear600.h b/include/configs/spear600.h new file mode 100755 index 0000000..755114d
...
+/* #define CONFIG_ETHADDR 00:11:22:33:44:55 */
+/* #define CONFIG_NETMASK 255.255.255.0 */ +/* #define CONFIG_IPADDR 192.168.1.10 */ +/* #define CONFIG_SERVERIP 192.168.1.1 */ +/* #define CONFIG_GATEWAYIP 192.168.1.1 */
Please remove.
Ok.removed commented code
Regards Vipin