Re: [PATCH 1/3] commands: Add MMC ext. CSD register tool
Hi Daniel,
Nice command. Several comments inline, there is still some way to go to
get this into shape.
On Mon, Aug 24, 2015 at 01:32:13PM +0200, Daniel Schultz wrote:
+
+static int print_field(u8 *reg, int index)
+{
+ int rev;
+ u32 val;
+ u32 tmp;
+ u64 tmp64;
+
+ rev = reg[EXT_CSD_REV];
+
+ if (rev = 7)
Additional print_field_v7/v6/v5() functions will reduce the indentation
level by one and help violating the 80 character limit less often.
+ switch (index) {
+ case EXT_CSD_CMDQ_MODE_EN:
+ print_field_caption(CMDQ_MODE_EN, RWE_P);
+ val = get_field_val(CMDQ_MODE_EN, 0, 0x1);
+ if (val)
+ printf(\tCommand queuing is enabled\n);
+ else
+ printf(\tCommand queuing is disabled\n);
Your printfs are very inefficient. You should use something like:
if (val)
str = en;
else
str = dis;
printf(Command queuing is %sabled\n, str);
Same goes for many other printfs. This will result in much less similar
strings in the binary.
+ return 1;
+
+ case EXT_CSD_SECURE_REMOVAL_TYPE:
+ print_field_caption(SECURE_REMOVAL_TYPE, RWaR);
+ val = get_field_val(SECURE_REMOVAL_TYPE, 0, 0xF);
+ switch (val) {
+ case 0x0:
+ printf(\t[3-0] Supported Secure Removal Type:
information removed by an erase of the physical memory\n);
+ break;
+ case 0x1:
+ printf(\t[3-0] Supported Secure Removal Type:
information removed by an overwriting the addressed locations with a
character followed by an erase\n);
+ break;
+ case 0x2:
+ printf(\t[3-0] Supported Secure Removal Type:
information removed by an overwriting the addressed locations with a
character, its complement, then a random character\n);
+ break;
+ case 0x3:
+ printf(\t[3-0] Supported Secure Removal Type:
information removed using a vendor defined\n);
+ break;
This is *very* verbose. Could you write this a bit more compact, maybe
case 0x0:
str = erase;
break;
case 0x1:
str = overwrite, then erase;
break;
case 0x2:
str = overwrite, complement, then random;
break;
case 0x3:
str = vendor defined;
break;
printf([3-0] Supported Secure Removal Type: %s\n, str);
Background is that this information only makes sense when being somewhat
familiar with the spec. Without knowing the spec at all even the more
verbose printfs do not help, but when knowing the spec a more compact
writing is enough to remember what is meant.
+
+static void print_register_raw(u8 *reg, int index)
+{
+ int i;
+
+ if (index == 0)
+ for (i = 0; i EXT_CSD_BLOCKSIZE; i++) {
+ if (i % 8 == 0)
+ printf(%u:, i);
+ printf( %#02x, reg[i]);
+ if (i % 8 == 7)
+ printf(\n);
+ }
memory_display(reg, 0, EXT_CSD_BLOCKSIZE, 1, 0);
Should do it here.
+static int do_extcsd(int argc, char *argv[])
+{
+ struct device_d *dev;
+ struct mci *mci;
+ struct mci_host *host;
+ u8 *dst;
+ int retval = 0;
+ int opt;
+ char*devname;
+ int index = 0;
+ int value = 0;
+ int write_operation = 0;
+ int always_write = 0;
+ int print_as_raw = 0;
+
+ if (argv[1] == NULL)
+ return COMMAND_ERROR_USAGE;
argc contains the number of arguments. When argc is 1 then argv[1] is
undefined. It may or may not be NULL in this case. You want to use if
(argc 2) here.
+
+ devname = argv[1];
You should use argv[optind] after parsing the arguments for the devname.
With this not only extcsd devname [OPTIONS] works but also extcsd
[OPTIONS] devname. Don't forget to check if there actually is
argv[optind] by
if (optind == argc)
return COMMAND_ERROR_USAGE;
+ if (!strncmp(devname, /dev/, 5))
+ devname += 5;
+
+ while ((opt = getopt(argc, argv, i:v:yr)) 0)
+ switch (opt) {
+ case 'i':
+ index = simple_strtoul(optarg, NULL, 0);
+ break;
+ case
Re: [PATCH 1/3] commands: Add MMC ext. CSD register tool
On Di, 2015-08-25 at 09:06 +0200, Sascha Hauer wrote: Your printfs are very inefficient. You should use something like: if (val) str = en; else str = dis; printf(Command queuing is %sabled\n, str); Same goes for many other printfs. This will result in much less similar strings in the binary. or like this: printf(Command queuing is %sabled\n, val ? en : dis); -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
[PATCH 1/3] commands: Add MMC ext. CSD register tool
This tools can read/write to the extended CSD register of MMC devices.
Signed-off-by: Daniel Schultz d.schu...@phytec.de
---
commands/Kconfig | 16 +
commands/Makefile |1 +
commands/extcsd.c | 2105 +
3 files changed, 2122 insertions(+)
create mode 100644 commands/extcsd.c
diff --git a/commands/Kconfig b/commands/Kconfig
index 133dcbf..293d685 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -242,6 +242,22 @@ config CMD_VERSION
barebox 2014.05.0-00142-gb289373 #177 Mon May 12 20:35:55 CEST 2014
+config CMD_EXTCSD
+ tristate
+ prompt read/write eMMC ext. CSD register
+ depends on MCI
+ help
+ Read or write the extended CSD register of a MMC device.
+
+ Usage: extcsd dev [-r | -i index [-r | -v value -y]]
+
+ Options:
+ -i field index of the register
+ -r print the register as raw data
+ -v value which will be written
+ -y don't request when writing to one time programmable
fields
+ __CAUTION__: this could damage the device!
+
# end Information commands
endmenu
diff --git a/commands/Makefile b/commands/Makefile
index 3d594c3..f2b1820 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -114,3 +114,4 @@ obj-$(CONFIG_CMD_STATE) += state.o
obj-$(CONFIG_CMD_DHCP) += dhcp.o
obj-$(CONFIG_CMD_DHRYSTONE)+= dhrystone.o
obj-$(CONFIG_CMD_SPD_DECODE) += spd_decode.o
+obj-$(CONFIG_CMD_EXTCSD) += extcsd.o
diff --git a/commands/extcsd.c b/commands/extcsd.c
new file mode 100644
index 000..79d1e97
--- /dev/null
+++ b/commands/extcsd.c
@@ -0,0 +1,2105 @@
+/*
+ *
+ * (C) Copyright 2015 Phytec Messtechnik GmbH
+ * Author: Daniel Schultz d.schu...@phytec.de
+ *
+ * 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 3 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.
+ *
+ */
+
+#include common.h
+#include command.h
+#include complete.h
+#include driver.h
+#include stdio.h
+#include mci.h
+#include getopt.h
+
+#define EXT_CSD_BLOCKSIZE 512
+
+/* Access types */
+#define R R
+#define RW R/W
+#define RWaR R/W R
+#define RWaRWE R/W R/W/E
+#define RWaRWC_P R/W R/W/C_P
+#define RWaRWC_PaRWE_P R/W, R/W/C_P R/W/E_P
+#define WE W/E
+#define RWER/W/E
+#define RWEaR R/W/E R
+#define RWEaRWE_P R/W/E R/W/E_P
+#define RWC_P R/W/C_P
+#define RWE_P R/W/E_P
+#define WE_P W/E_P
+
+#define print_field_caption(reg_name, access_mode)\
+ do { \
+ printf(#reg_name[%u]:\n, EXT_CSD_##reg_name);\
+ printf(\tValue: %#02x\n, reg[index]);\
+ printf(\tAccess: access_mode\n); \
+ } while (false);
+
+#define print_field_caption_with_offset(reg_name, offset, access_mode)\
+ do { \
+ printf(#reg_name[%u]:\n, EXT_CSD_##reg_name + offset); \
+ printf(\tValue: %#02x\n, reg[index]);\
+ printf(\tAccess: access_mode\n); \
+ } while (false);
+
+#define get_field_val(reg_name, offset, mask) \
+ ((reg[EXT_CSD_##reg_name] offset) mask)
+
+#define get_field_val_with_index(index, offset, mask) \
+ ((reg[index] offset) mask)
+
+static void print_access_type_key(void)
+{
+ printf(\nR: Read only.\n);
+ printf(W: One time programmable and not readable.\n);
+ printf(R/W: One time programmable and readable.\n);
+ printf(W/E: Multiple writable with value kept after power failure,
H/W reset assertion and any CMD0 reset and not readable.\n);
+ printf(R/W/E: Multiple writable with value kept after power failure,
H/W reset assertion and any CMD0 reset and readable.\n);
+ printf(R/W/C_P: Writable after value cleared by power failure and
HW/rest assertion (the value not cleared by CMD0 reset) and readable.\n);
+ printf(R/W/E_P: Multiple writable with value reset after power
failure, H/W reset assertion and any CMD0 reset and readable.\n);
+ printf(W/E_P: Multiple writable with value reset after power
failure, H/W reset assertion and any CMD0 reset and
