Skip to contentSkip to conversation

disk: make get_partition_info() always available to disk.c

PATCH Superseded Stephen Warren 2 participants 4 messages Sep 21, 2012 – Sep 25, 2012

lore
Share & tools
lore

Stephen Warren

author
#0 lore raw

PATCH v2 1/1 disk: make get_partition_info() always available to disk.c

From: Stephen Warren <[email protected]>

Now that get_device_and_partition() always calls get_partition_info() when disk.c is compiled, we must always compile the function, rather than ifdef it away.

The implementation must be conditional based on CONFIG_CMD_* etc., since that's what e.g. part_dos.c uses to ifdef out get_partition_info_dos(); CONFIG_DOS_PARTITION can be enabled even without those commands being enabled.

Technically, this change is required before Rob's "disk/part: introduce get_device_and_partition" patch. However, at least when the compiler optimizer is turned on, it isn't required before then in practice, since get_device_and_partition() calls get_dev(), which is stubbed out in disk.c under exactly the same conditions that get_partition_info() is not compiled, and hence the compiler never generates code for the call to the missing function. However, in my later patch "disk:
get_device_and_partition() "auto" partition and cleanup", the optimizer doesn't succeed at this, and may attempt to reference the undefined function.

Signed-off-by Stephen Warren <[email protected]>

---
v2: Add CONFIG_CMD_* etc. ifdefs around the implementation.

Rob, I wonder if you shouldn't squash this into your series. Then, I'll need to rebase mine on your again since this causes a few nasty conflicts with my series.

---
 disk/part.c |  118 +++++++++++++++++++++++++++++++---------------------------
 1 files changed, 63 insertions(+), 55 deletions(-)
disk/part.c +63 -55
index 76f3939..f659cc3 100644
@@ -291,61 +291,6 @@ void init_part (block_dev_desc_t * dev_desc)
 }
 
 
-int get_partition_info (block_dev_desc_t *dev_desc, int part
-					, disk_partition_t *info)
-{
-	switch (dev_desc->part_type) {
-#ifdef CONFIG_MAC_PARTITION
-	case PART_TYPE_MAC:
-		if (get_partition_info_mac(dev_desc,part,info) == 0) {
-			PRINTF ("## Valid MAC partition found ##\n");
-			return (0);
-		}
-		break;
-#endif
-
-#ifdef CONFIG_DOS_PARTITION
-	case PART_TYPE_DOS:
-		if (get_partition_info_dos(dev_desc,part,info) == 0) {
-			PRINTF ("## Valid DOS partition found ##\n");
-			return (0);
-		}
-		break;
-#endif
-
-#ifdef CONFIG_ISO_PARTITION
-	case PART_TYPE_ISO:
-		if (get_partition_info_iso(dev_desc,part,info) == 0) {
-			PRINTF ("## Valid ISO boot partition found ##\n");
-			return (0);
-		}
-		break;
-#endif
-
-#ifdef CONFIG_AMIGA_PARTITION
-	case PART_TYPE_AMIGA:
-	    if (get_partition_info_amiga(dev_desc, part, info) == 0)
-	    {
-		PRINTF ("## Valid Amiga partition found ##\n");
-		return (0);
-	    }
-	    break;
-#endif
-
-#ifdef CONFIG_EFI_PARTITION
-	case PART_TYPE_EFI:
-		if (get_partition_info_efi(dev_desc,part,info) == 0) {
-			PRINTF ("## Valid EFI partition found ##\n");
-			return (0);
-		}
-		break;
-#endif
-	default:
-		break;
-	}
-	return (-1);
-}
-
 static void print_part_header (const char *type, block_dev_desc_t * dev_desc)
 {
 	puts ("\nPartition Map for ");

@@ -433,3 +378,66 @@ void print_part (block_dev_desc_t * dev_desc)
 #endif
 
 #endif
+
+int get_partition_info(block_dev_desc_t *dev_desc, int part
+					, disk_partition_t *info)
+{
+#if defined(CONFIG_CMD_IDE) || \
+	defined(CONFIG_CMD_SATA) || \
+	defined(CONFIG_CMD_SCSI) || \
+	defined(CONFIG_CMD_USB) || \
+	defined(CONFIG_MMC) || \
+	defined(CONFIG_SYSTEMACE)
+
+	switch (dev_desc->part_type) {
+#ifdef CONFIG_MAC_PARTITION
+	case PART_TYPE_MAC:
+		if (get_partition_info_mac(dev_desc, part, info) == 0) {
+			PRINTF("## Valid MAC partition found ##\n");
+			return 0;
+		}
+		break;
+#endif
+
+#ifdef CONFIG_DOS_PARTITION
+	case PART_TYPE_DOS:
+		if (get_partition_info_dos(dev_desc, part, info) == 0) {
+			PRINTF("## Valid DOS partition found ##\n");
+			return 0;
+		}
+		break;
+#endif
+
+#ifdef CONFIG_ISO_PARTITION
+	case PART_TYPE_ISO:
+		if (get_partition_info_iso(dev_desc, part, info) == 0) {
+			PRINTF("## Valid ISO boot partition found ##\n");
+			return 0;
+		}
+		break;
+#endif
+
+#ifdef CONFIG_AMIGA_PARTITION
+	case PART_TYPE_AMIGA:
+		if (get_partition_info_amiga(dev_desc, part, info) == 0) {
+			PRINTF("## Valid Amiga partition found ##\n");
+			return 0;
+		}
+		break;
+#endif
+
+#ifdef CONFIG_EFI_PARTITION
+	case PART_TYPE_EFI:
+		if (get_partition_info_efi(dev_desc, part, info) == 0) {
+			PRINTF("## Valid EFI partition found ##\n");
+			return 0;
+		}
+		break;
+#endif
+	default:
+		break;
+	}
+#endif
+
+	return -1;
+}
Signature · 1 line
1.7.0.4
#1 lore raw

PATCH v2 1/1 disk: make get_partition_info() always available to disk.c

Quoted 22 lines · Stephen Warren

From: Stephen Warren <[email protected]>

Now that get_device_and_partition() always calls get_partition_info() when disk.c is compiled, we must always compile the function, rather than ifdef it away.

The implementation must be conditional based on CONFIG_CMD_* etc., since that's what e.g. part_dos.c uses to ifdef out get_partition_info_dos(); CONFIG_DOS_PARTITION can be enabled even without those commands being enabled.

Technically, this change is required before Rob's "disk/part: introduce get_device_and_partition" patch. However, at least when the compiler optimizer is turned on, it isn't required before then in practice, since get_device_and_partition() calls get_dev(), which is stubbed out in disk.c under exactly the same conditions that get_partition_info() is not compiled, and hence the compiler never generates code for the call to the missing function. However, in my later patch "disk:
get_device_and_partition() "auto" partition and cleanup", the optimizer doesn't succeed at this, and may attempt to reference the undefined function.

Quoted 7 lines
Signed-off-by Stephen Warren <[email protected]>

---
v2: Add CONFIG_CMD_* etc. ifdefs around the implementation.

Rob, I wonder if you shouldn't squash this into your series. Then, I'll need to rebase mine on your again since this causes a few nasty conflicts with my series.

I _really_ want to see incremental changes. It's one of the good practices of the kernel folks and I'd like to see us do it as well as much as we can.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120921/1838f7da/attachment.pgp>
author
#2 lore raw

PATCH v2 1/1 disk: make get_partition_info() always available to disk.c

Quoted 27 lines · Tom Rini
Quoted 26 lines · Stephen Warren

From: Stephen Warren <[email protected]>

Now that get_device_and_partition() always calls get_partition_info() when disk.c is compiled, we must always compile the function, rather than ifdef it away.

The implementation must be conditional based on CONFIG_CMD_* etc., since that's what e.g. part_dos.c uses to ifdef out get_partition_info_dos(); CONFIG_DOS_PARTITION can be enabled even without those commands being enabled.

Technically, this change is required before Rob's "disk/part:
introduce get_device_and_partition" patch. However, at least when the compiler optimizer is turned on, it isn't required before then in practice, since get_device_and_partition() calls get_dev(), which is stubbed out in disk.c under exactly the same conditions that get_partition_info() is not compiled, and hence the compiler never generates code for the call to the missing function. However, in my later patch "disk:
get_device_and_partition() "auto" partition and cleanup", the optimizer doesn't succeed at this, and may attempt to reference the undefined function.

Signed-off-by Stephen Warren <[email protected]> --- v2: Add

CONFIG_CMD_* etc. ifdefs around the implementation.

Quoted 7 lines
Quoted 3 lines

Rob, I wonder if you shouldn't squash this into your series. Then, I'll need to rebase mine on your again since this causes a few nasty conflicts with my series.

I _really_ want to see incremental changes. It's one of the good practices of the kernel folks and I'd like to see us do it as well as much as we can.

OK, well in that case, you can just apply the patch standalone before you apply Rob's series then. I think we'll both need to rebase to avoid conflict issues those - e.g. I edited the function that got moved in my patch series and had to manually re-apply the change to the new code location after I created this patch.

#3 lore raw

PATCH v2 1/1 disk: make get_partition_info() always available to disk.c

In reply to Stephen Warren
Quoted 22 lines · Stephen Warren

From: Stephen Warren <[email protected]>

Now that get_device_and_partition() always calls get_partition_info() when disk.c is compiled, we must always compile the function, rather than ifdef it away.

The implementation must be conditional based on CONFIG_CMD_* etc., since that's what e.g. part_dos.c uses to ifdef out get_partition_info_dos(); CONFIG_DOS_PARTITION can be enabled even without those commands being enabled.

Technically, this change is required before Rob's "disk/part: introduce get_device_and_partition" patch. However, at least when the compiler optimizer is turned on, it isn't required before then in practice, since get_device_and_partition() calls get_dev(), which is stubbed out in disk.c under exactly the same conditions that get_partition_info() is not compiled, and hence the compiler never generates code for the call to the missing function. However, in my later patch "disk:
get_device_and_partition() "auto" partition and cleanup", the optimizer doesn't succeed at this, and may attempt to reference the undefined function.

Quoted 1 line
Signed-off-by Stephen Warren <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120925/b67fb6df/attachment.pgp>