Skip to contentSkip to conversation

blk: Use macros for block device vendor/product/rev string size

PATCH Superseded Bin Meng 2 participants 5 messages Sep 4, 2017 – Sep 6, 2017 Block layer

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Bin Meng

author
#0 lore raw

PATCH 1/4 blk: Use macros for block device vendor/product/rev string size

So far these are using magic numbers. Replace them with macros.

---

 include/blk.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
include/blk.h +7 -3
index a106f9c..fced138 100644
@@ -36,6 +36,10 @@ enum if_type {
 	IF_TYPE_COUNT,			/* Number of interface types */
 };
 
+#define BLK_VEN_SIZE		41
+#define BLK_PRD_SIZE		21
+#define BLK_REV_SIZE		9
+
 /*
  * With driver model (CONFIG_BLK) this is uclass platform data, accessible
  * with dev_get_uclass_platdata(dev)
@@ -60,9 +64,9 @@ struct blk_desc {
 	lbaint_t	lba;		/* number of blocks */
 	unsigned long	blksz;		/* block size */
 	int		log2blksz;	/* for convenience: log2(blksz) */
-	char		vendor[40+1];	/* IDE model, SCSI Vendor */
-	char		product[20+1];	/* IDE Serial no, SCSI product */
-	char		revision[8+1];	/* firmware revision */
+	char		vendor[BLK_VEN_SIZE];	/* device vendor string */
+	char		product[BLK_PRD_SIZE];	/* device product number */
+	char		revision[BLK_REV_SIZE];	/* firmware revision */
 #if CONFIG_IS_ENABLED(BLK)
 	/*
 	 * For now we have a few functions which take struct blk_desc as a
Signature · 1 line
2.9.2
author
#1 lore raw

PATCH 2/4 block: ide: Fix block read/write with driver model

This converts the IDE driver to driver model so that block read and write are fully functional.

Fixes b7c6baef ("x86: Convert MMC to driver model")

---

 drivers/block/ide.c    | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/dm/uclass-id.h |  1 +
 2 files changed, 69 insertions(+)
drivers/block/ide.c +68 -0
index ce51153..58b295e 100644
@@ -827,12 +827,20 @@ void ide_init(void)
 		ide_ident(&ide_dev_desc[i]);
 		dev_print(&ide_dev_desc[i]);
 
+#ifndef CONFIG_BLK
 		if ((ide_dev_desc[i].lba > 0) && (ide_dev_desc[i].blksz > 0)) {
 			/* initialize partition type */
 			part_init(&ide_dev_desc[i]);
 		}
+#endif
 	}
 	WATCHDOG_RESET();
+
+#ifdef CONFIG_BLK
+	struct udevice *dev;
+
+	uclass_first_device(UCLASS_IDE, &dev);
+#endif
 }
 
 /* We only need to swap data if we are running on a big endian cpu. */
@@ -1147,6 +1155,21 @@ int ide_device_present(int dev)
 #endif
 
 #ifdef CONFIG_BLK
+static int ide_blk_probe(struct udevice *udev)
+{
+	struct blk_desc *desc = dev_get_uclass_platdata(udev);
+	int devnum = desc->devnum;
+
+	/* fill in device vendor/product/rev strings */
+	strncpy(desc->vendor, ide_dev_desc[devnum].vendor, BLK_VEN_SIZE);
+	strncpy(desc->product, ide_dev_desc[devnum].product, BLK_PRD_SIZE);
+	strncpy(desc->revision, ide_dev_desc[devnum].revision, BLK_REV_SIZE);
+
+	part_init(desc);
+
+	return 0;
+}
+
 static const struct blk_ops ide_blk_ops = {
 	.read	= ide_read,
 	.write	= ide_write,
@@ -1156,6 +1179,51 @@ U_BOOT_DRIVER(ide_blk) = {
 	.name		= "ide_blk",
 	.id		= UCLASS_BLK,
 	.ops		= &ide_blk_ops,
+	.probe		= ide_blk_probe,
+};
+
+static int ide_probe(struct udevice *udev)
+{
+	struct udevice *blk_dev;
+	char name[20];
+	int blksz;
+	lbaint_t size;
+	int i;
+	int ret;
+
+	for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
+		if (ide_dev_desc[i].type != DEV_TYPE_UNKNOWN) {
+			sprintf(name, "blk#%d", i);
+
+			blksz = ide_dev_desc[i].blksz;
+			size = blksz * ide_dev_desc[i].lba;
+			ret = blk_create_devicef(udev, "ide_blk", name,
+						 IF_TYPE_IDE, i,
+						 blksz, size, &blk_dev);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+U_BOOT_DRIVER(ide) = {
+	.name		= "ide",
+	.id		= UCLASS_IDE,
+	.probe		= ide_probe,
+};
+
+struct pci_device_id ide_supported[] = {
+	{ PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_IDE << 8, 0xffff00) },
+	{ }
+};
+
+U_BOOT_PCI_DEVICE(ide, ide_supported);
+
+UCLASS_DRIVER(ide) = {
+	.name		= "ide",
+	.id		= UCLASS_IDE,
 };
 #else
 U_BOOT_LEGACY_BLK(ide) = {
include/dm/uclass-id.h +1 -0
index 1a50199..3fc2083 100644
@@ -41,6 +41,7 @@ enum uclass_id {
 	UCLASS_I2C_EEPROM,	/* I2C EEPROM device */
 	UCLASS_I2C_GENERIC,	/* Generic I2C device */
 	UCLASS_I2C_MUX,		/* I2C multiplexer */
+	UCLASS_IDE,		/* IDE device */
 	UCLASS_IRQ,		/* Interrupt controller */
 	UCLASS_KEYBOARD,	/* Keyboard input device */
 	UCLASS_LED,		/* Light-emitting diode (LED) */
Signature · 1 line
2.9.2
author
#2 lore raw

PATCH 3/4 block: ide: Don't bother to create BLK device if no CDROM inserted

In reply to Bin Meng

When there is no CDROM inserted, the block size is zero hence there is no need to create a BLK device for it.

---

 drivers/block/ide.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/block/ide.c +7 -0
index 58b295e..8125ff8 100644
@@ -1197,6 +1197,13 @@ static int ide_probe(struct udevice *udev)
 
 			blksz = ide_dev_desc[i].blksz;
 			size = blksz * ide_dev_desc[i].lba;
+
+			/*
+			 * With CDROM, if there is no CD inserted, blksz will
+			 * be zero, don't bother to create IDE block device.
+			 */
+			if (!blksz)
+				continue;
 			ret = blk_create_devicef(udev, "ide_blk", name,
 						 IF_TYPE_IDE, i,
 						 blksz, size, &blk_dev);
Signature · 1 line
2.9.2
author
#3 lore raw

PATCH 4/4 cmd: ide: Make the first device the default one

In reply to Bin Meng

At present the IDE device number is initialized to -1, which means we cannot type "ide read" command before setting the device number via "ide device #".

For convenience, let's set the first device as the default one.

---

 cmd/ide.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
cmd/ide.c +1 -1
index e3c3242..bdb5980 100644
@@ -30,7 +30,7 @@
 #endif
 
 /* Current I/O Device	*/
-static int curr_device = -1;
+static int curr_device;
 
 int do_ide(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
 {
Signature · 1 line
2.9.2
#4 lore raw

PATCH 1/4 blk: Use macros for block device vendor/product/rev string size

In reply to Bin Meng
Quoted 24 lines · Bin Meng

So far these are using magic numbers. Replace them with macros.

---

 include/blk.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
include/blk.h +4 -0
index a106f9c..fced138 100644
@@ -36,6 +36,10 @@ enum if_type {
 	IF_TYPE_COUNT,			/* Number of interface types */
 };
 
+#define BLK_VEN_SIZE		41
+#define BLK_PRD_SIZE		21
+#define BLK_REV_SIZE		9
+
 /*
  * With driver model (CONFIG_BLK) this is uclass platform data, accessible
  * with dev_get_uclass_platdata(dev)

include/blk.h struct blk_desc { · line 69

Quoted 9 lines
include/blk.h +3 -3
@@ -60,9 +64,9 @@ struct blk_desc {
 	lbaint_t	lba;		/* number of blocks */
 	unsigned long	blksz;		/* block size */
 	int		log2blksz;	/* for convenience: log2(blksz) */
-	char		vendor[40+1];	/* IDE model, SCSI Vendor */
-	char		product[20+1];	/* IDE Serial no, SCSI product */
-	char		revision[8+1];	/* firmware revision */
+	char		vendor[BLK_VEN_SIZE];	/* device vendor string */
+	char		product[BLK_PRD_SIZE];	/* device product number */
+	char		revision[BLK_REV_SIZE];	/* firmware revision */

I think we should be using BLK_xxx_SIZE + 1 instead here and then it looks like at least 2/4 shows why, we start to copy to the end of a char buffer in, rather than ensuring we have string+NULL in each. That is why we use SIZE+1 here. Thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170905/7e4f6490/attachment.sig>