dfu: mmc: raw data write fix

When user attempted to perform a raw write using DFU (vide dfu_fill_entity_mmc) with MMC interface not initialized before, get_mmc_blk_size() reported invalid (zero) block size - it wasn't possible to write ie. a new u-boot image. This commit fixes that by initializing MMC device before use in dfu_fill_entity_mmc(). While fixing initialization sequence, I had to change about half of dfu_fill_entity_mmc's body, so I refactored it on the way to make it, IMHO, considerably more comprehensible. Being left as dead code, get_mmc_blk_size() was removed. Tested on Samsung Goni. Signed-off-by: Mateusz Zalega <m.zalega@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> Acked-by: Lukasz Majewski <l.majewski@samsung.com> Acked-by: Tom Rini <trini@ti.com> Cc: Minkyu Kang <mk7.kang@samsung.com>

dfu: mmc: raw data write fix
When user attempted to perform a raw write using DFU (vide dfu_fill_entity_mmc) with MMC interface not initialized before, get_mmc_blk_size() reported invalid (zero) block size - it wasn't possible to write ie. a new u-boot image. This commit fixes that by initializing MMC device before use in dfu_fill_entity_mmc(). While fixing initialization sequence, I had to change about half of dfu_fill_entity_mmc's body, so I refactored it on the way to make it, IMHO, considerably more comprehensible. Being left as dead code, get_mmc_blk_size() was removed. Tested on Samsung Goni. Signed-off-by: Mateusz Zalega <m.zalega@samsung.com> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> Acked-by: Lukasz Majewski <l.majewski@samsung.com> Acked-by: Tom Rini <trini@ti.com> Cc: Minkyu Kang <mk7.kang@samsung.com>
Mateusz Zalega · Lukasz Majewski
1 parent 75504e9592
Showing 2 changed files with 65 additions and 45 deletions Side-by-side Diff
drivers/dfu/dfu_mmc.c
include/dfu.h
@@ -184,66 +184,91 @@
 	return ret;
 }
  
+/*
+ * @param s Parameter string containing space-separated arguments:
+ *	1st:
+ *		raw	(raw read/write)
+ *		fat	(files)
+ *		ext4	(^)
+ *		part	(partition image)
+ *	2nd and 3rd:
+ *		lba_start and lba_size, for raw write
+ *		mmc_dev and mmc_part, for filesystems and part
+ */
 int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s)
 {
-	int dev, part;
+	const char *entity_type;
+	size_t second_arg;
+	size_t third_arg;
+
 	struct mmc *mmc;
-	block_dev_desc_t *blk_dev;
-	disk_partition_t partinfo;
-	char *st;
  
-	dfu->dev_type = DFU_DEV_MMC;
-	st = strsep(&s, " ");
-	if (!strcmp(st, "mmc")) {
-		dfu->layout = DFU_RAW_ADDR;
-		dfu->data.mmc.lba_start = simple_strtoul(s, &s, 16);
-		dfu->data.mmc.lba_size = simple_strtoul(++s, &s, 16);
-		dfu->data.mmc.lba_blk_size = get_mmc_blk_size(dfu->dev_num);
-	} else if (!strcmp(st, "fat")) {
-		dfu->layout = DFU_FS_FAT;
-	} else if (!strcmp(st, "ext4")) {
-		dfu->layout = DFU_FS_EXT4;
-	} else if (!strcmp(st, "part")) {
+	const char *argv[3];
+	const char **parg = argv;
  
-		dfu->layout = DFU_RAW_ADDR;
-
-		dev = simple_strtoul(s, &s, 10);
-		s++;
-		part = simple_strtoul(s, &s, 10);
-
-		mmc = find_mmc_device(dev);
-		if (mmc == NULL || mmc_init(mmc)) {
-			printf("%s: could not find mmc device #%d!\n",
-			       __func__, dev);
+	for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) {
+		*parg = strsep(&s, " ");
+		if (*parg == NULL) {
+			error("Invalid number of arguments.\n");
 			return -ENODEV;
 		}
+	}
  
-		blk_dev = &mmc->block_dev;
-		if (get_partition_info(blk_dev, part, &partinfo) != 0) {
-			printf("%s: could not find partition #%d on mmc device #%d!\n",
-			       __func__, part, dev);
+	entity_type = argv[0];
+	second_arg = simple_strtoul(argv[1], NULL, 16);
+	third_arg = simple_strtoul(argv[2], NULL, 16);
+
+	mmc = find_mmc_device(dfu->dev_num);
+	if (mmc == NULL) {
+		error("Couldn't find MMC device no. %d.\n", dfu->dev_num);
+		return -ENODEV;
+	}
+
+	if (mmc_init(mmc)) {
+		error("Couldn't init MMC device.\n");
+		return -ENODEV;
+	}
+
+	if (!strcmp(entity_type, "raw")) {
+		dfu->layout			= DFU_RAW_ADDR;
+		dfu->data.mmc.lba_start		= second_arg;
+		dfu->data.mmc.lba_size		= third_arg;
+		dfu->data.mmc.lba_blk_size	= mmc->read_bl_len;
+	} else if (!strcmp(entity_type, "part")) {
+		disk_partition_t partinfo;
+		block_dev_desc_t *blk_dev = &mmc->block_dev;
+		int mmcdev = second_arg;
+		int mmcpart = third_arg;
+
+		if (get_partition_info(blk_dev, mmcpart, &partinfo) != 0) {
+			error("Couldn't find part #%d on mmc device #%d\n",
+			      mmcpart, mmcdev);
 			return -ENODEV;
 		}
  
-		dfu->data.mmc.lba_start = partinfo.start;
-		dfu->data.mmc.lba_size = partinfo.size;
-		dfu->data.mmc.lba_blk_size = partinfo.blksz;
-
+		dfu->layout			= DFU_RAW_ADDR;
+		dfu->data.mmc.lba_start		= partinfo.start;
+		dfu->data.mmc.lba_size		= partinfo.size;
+		dfu->data.mmc.lba_blk_size	= partinfo.blksz;
+	} else if (!strcmp(entity_type, "fat")) {
+		dfu->layout = DFU_FS_FAT;
+	} else if (!strcmp(entity_type, "ext4")) {
+		dfu->layout = DFU_FS_EXT4;
 	} else {
-		printf("%s: Memory layout (%s) not supported!\n", __func__, st);
+		error("Memory layout (%s) not supported!\n", entity_type);
 		return -ENODEV;
 	}
  
-	if (dfu->layout == DFU_FS_EXT4 || dfu->layout == DFU_FS_FAT) {
-		dfu->data.mmc.dev = simple_strtoul(s, &s, 10);
-		dfu->data.mmc.part = simple_strtoul(++s, &s, 10);
+	/* if it's NOT a raw write */
+	if (strcmp(entity_type, "raw")) {
+		dfu->data.mmc.dev = second_arg;
+		dfu->data.mmc.part = third_arg;
 	}
  
+	dfu->dev_type = DFU_DEV_MMC;
 	dfu->read_medium = dfu_read_medium_mmc;
 	dfu->write_medium = dfu_write_medium_mmc;
 	dfu->flush_medium = dfu_flush_medium_mmc;
-
-	/* initial state */
 	dfu->inited = 0;
  
 	return 0;
@@ -64,11 +64,6 @@
 	unsigned int	size;
 };
  
-static inline unsigned int get_mmc_blk_size(int dev)
-{
-	return find_mmc_device(dev)->read_bl_len;
-}
-
 #define DFU_NAME_SIZE			32
 #define DFU_CMD_BUF_SIZE		128
 #ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE
...	...	@@ -184,66 +184,91 @@
184	184	return ret;
185	185	}
186	186
	187	+/*
	188	+ * @param s Parameter string containing space-separated arguments:
	189	+ * 1st:
	190	+ * raw (raw read/write)
	191	+ * fat (files)
	192	+ * ext4 (^)
	193	+ * part (partition image)
	194	+ * 2nd and 3rd:
	195	+ * lba_start and lba_size, for raw write
	196	+ * mmc_dev and mmc_part, for filesystems and part
	197	+ */
187	198	int dfu_fill_entity_mmc(struct dfu_entity dfu, char s)
188	199	{
189		- int dev, part;
	200	+ const char *entity_type;
	201	+ size_t second_arg;
	202	+ size_t third_arg;
	203	+
190	204	struct mmc *mmc;
191		- block_dev_desc_t *blk_dev;
192		- disk_partition_t partinfo;
193		- char *st;
194	205
195		- dfu->dev_type = DFU_DEV_MMC;
196		- st = strsep(&s, " ");
197		- if (!strcmp(st, "mmc")) {
198		- dfu->layout = DFU_RAW_ADDR;
199		- dfu->data.mmc.lba_start = simple_strtoul(s, &s, 16);
200		- dfu->data.mmc.lba_size = simple_strtoul(++s, &s, 16);
201		- dfu->data.mmc.lba_blk_size = get_mmc_blk_size(dfu->dev_num);
202		- } else if (!strcmp(st, "fat")) {
203		- dfu->layout = DFU_FS_FAT;
204		- } else if (!strcmp(st, "ext4")) {
205		- dfu->layout = DFU_FS_EXT4;
206		- } else if (!strcmp(st, "part")) {
	206	+ const char *argv[3];
	207	+ const char **parg = argv;
207	208
208		- dfu->layout = DFU_RAW_ADDR;
209		-
210		- dev = simple_strtoul(s, &s, 10);
211		- s++;
212		- part = simple_strtoul(s, &s, 10);
213		-
214		- mmc = find_mmc_device(dev);
215		- if (mmc == NULL \|\| mmc_init(mmc)) {
216		- printf("%s: could not find mmc device #%d!\n",
217		- __func__, dev);
	209	+ for (; parg < argv + sizeof(argv) / sizeof(*argv); ++parg) {
	210	+ *parg = strsep(&s, " ");
	211	+ if (*parg == NULL) {
	212	+ error("Invalid number of arguments.\n");
218	213	return -ENODEV;
219	214	}
	215	+ }
220	216
221		- blk_dev = &mmc->block_dev;
222		- if (get_partition_info(blk_dev, part, &partinfo) != 0) {
223		- printf("%s: could not find partition #%d on mmc device #%d!\n",
224		- __func__, part, dev);
	217	+ entity_type = argv[0];
	218	+ second_arg = simple_strtoul(argv[1], NULL, 16);
	219	+ third_arg = simple_strtoul(argv[2], NULL, 16);
	220	+
	221	+ mmc = find_mmc_device(dfu->dev_num);
	222	+ if (mmc == NULL) {
	223	+ error("Couldn't find MMC device no. %d.\n", dfu->dev_num);
	224	+ return -ENODEV;
	225	+ }
	226	+
	227	+ if (mmc_init(mmc)) {
	228	+ error("Couldn't init MMC device.\n");
	229	+ return -ENODEV;
	230	+ }
	231	+
	232	+ if (!strcmp(entity_type, "raw")) {
	233	+ dfu->layout = DFU_RAW_ADDR;
	234	+ dfu->data.mmc.lba_start = second_arg;
	235	+ dfu->data.mmc.lba_size = third_arg;
	236	+ dfu->data.mmc.lba_blk_size = mmc->read_bl_len;
	237	+ } else if (!strcmp(entity_type, "part")) {
	238	+ disk_partition_t partinfo;
	239	+ block_dev_desc_t *blk_dev = &mmc->block_dev;
	240	+ int mmcdev = second_arg;
	241	+ int mmcpart = third_arg;
	242	+
	243	+ if (get_partition_info(blk_dev, mmcpart, &partinfo) != 0) {
	244	+ error("Couldn't find part #%d on mmc device #%d\n",
	245	+ mmcpart, mmcdev);
225	246	return -ENODEV;
226	247	}
227	248
228		- dfu->data.mmc.lba_start = partinfo.start;
229		- dfu->data.mmc.lba_size = partinfo.size;
230		- dfu->data.mmc.lba_blk_size = partinfo.blksz;
231		-
	249	+ dfu->layout = DFU_RAW_ADDR;
	250	+ dfu->data.mmc.lba_start = partinfo.start;
	251	+ dfu->data.mmc.lba_size = partinfo.size;
	252	+ dfu->data.mmc.lba_blk_size = partinfo.blksz;
	253	+ } else if (!strcmp(entity_type, "fat")) {
	254	+ dfu->layout = DFU_FS_FAT;
	255	+ } else if (!strcmp(entity_type, "ext4")) {
	256	+ dfu->layout = DFU_FS_EXT4;
232	257	} else {
233		- printf("%s: Memory layout (%s) not supported!\n", __func__, st);
	258	+ error("Memory layout (%s) not supported!\n", entity_type);
234	259	return -ENODEV;
235	260	}
236	261
237		- if (dfu->layout == DFU_FS_EXT4 \|\| dfu->layout == DFU_FS_FAT) {
238		- dfu->data.mmc.dev = simple_strtoul(s, &s, 10);
239		- dfu->data.mmc.part = simple_strtoul(++s, &s, 10);
	262	+ /* if it's NOT a raw write */
	263	+ if (strcmp(entity_type, "raw")) {
	264	+ dfu->data.mmc.dev = second_arg;
	265	+ dfu->data.mmc.part = third_arg;
240	266	}
241	267
	268	+ dfu->dev_type = DFU_DEV_MMC;
242	269	dfu->read_medium = dfu_read_medium_mmc;
243	270	dfu->write_medium = dfu_write_medium_mmc;
244	271	dfu->flush_medium = dfu_flush_medium_mmc;
245		-
246		- /* initial state */
247	272	dfu->inited = 0;
248	273
249	274	return 0;
...	...	@@ -64,11 +64,6 @@
64	64	unsigned int size;
65	65	};
66	66
67		-static inline unsigned int get_mmc_blk_size(int dev)
68		-{
69		- return find_mmc_device(dev)->read_bl_len;
70		-}
71		-
72	67	#define DFU_NAME_SIZE 32
73	68	#define DFU_CMD_BUF_SIZE 128
74	69	#ifndef CONFIG_SYS_DFU_DATA_BUF_SIZE