[PATCH 00/30] ide: Clean up code and fix a few bugs

[PATCH 00/30] ide: Clean up code and fix a few bugs

[Simon Glass]

This code was converted to driver model a long time again but it was a
pretty rough conversion. It introduced a few minor bugs, e.g. the device
capacity is incorrect and some flags are lost (such as lba48).

This series tidies up the code and fixes these bugs. This involves quite
a bit of refactoring, so it is done one patch at a time for easier
review.


Simon Glass (30):
  ide: Move ATA_CURR_BASE to C file
  ide: Use mdelay() for long delays
  ide: Drop CONFIG_START_IDE
  ide: Drop init for not using BLK
  ide: Move ide_init() into probing
  ide: Drop ide_device_present()
  ide: Move a few functions further up the file
  ide: Drop weak functions
  ide: Create a prototype for ide_set_reset()
  ide: Correct use of ATAPI
  ide: Make function static
  ide: Change the retries variable
  ide: Refactor confusing loop code
  ide: Simplify success condition
  ide: Avoid preprocessor for CONFIG_ATAPI
  ide: Avoid preprocessor for CONFIG_LBA48
  ide: Move bus init into a function
  ide: Make ide_bus_ok[] a local variable
  ide: Move setting of vendor strings into ide_probe()
  ide: Move ide_init() entirely within ide_probe()
  ide: Combine the two loops in ide_probe()
  ide: Use desc consistently for struct blk_desc
  ide: Make ide_ident() return an error code
  ide: Move all blk_desc init into ide_ident()
  ide: Use a single local blk_desc for ide_ident()
  ide: Correct LBA setting
  ide: Tidy up ide_reset()
  ide: Convert to use log_debug()
  ide: Simplify expressions and hex values
  ide: Make use of U-Boot types

 cmd/ide.c           |  22 +-
 common/board_r.c    |  17 -
 drivers/block/ide.c | 841 ++++++++++++++++++++------------------------
 include/blk.h       |   5 +-
 include/ide.h       |  48 +--
 test/boot/bootdev.c |   2 -
 6 files changed, 415 insertions(+), 520 deletions(-)

-- 
2.40.0.348.gf938b09366-goog

[PATCH 01/30] ide: Move ATA_CURR_BASE to C file

[Simon Glass]

This is not used outside one C file. Move it out of the header to
reduce its visbility.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 3 +++
 include/ide.h       | 3 ---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 1ad9b6c1267e..f36bec8b3a8a 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -36,6 +36,9 @@ ulong ide_bus_offset[CONFIG_SYS_IDE_MAXBUS] = {
 #endif
 };
 
+#define ATA_CURR_BASE(dev)	(CONFIG_SYS_ATA_BASE_ADDR + \
+		ide_bus_offset[IDE_BUS(dev)])
+
 static int ide_bus_ok[CONFIG_SYS_IDE_MAXBUS];
 
 struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
diff --git a/include/ide.h b/include/ide.h
index 426cef4e39e0..58f6640c61be 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -11,9 +11,6 @@
 
 #define IDE_BUS(dev)	(dev / (CONFIG_SYS_IDE_MAXDEVICE / CONFIG_SYS_IDE_MAXBUS))
 
-#define	ATA_CURR_BASE(dev)	(CONFIG_SYS_ATA_BASE_ADDR+ide_bus_offset[IDE_BUS(dev)])
-extern ulong ide_bus_offset[];
-
 /*
  * Function Prototypes
  */
-- 
2.40.0.348.gf938b09366-goog

[PATCH 02/30] ide: Use mdelay() for long delays

[Simon Glass]

Rather than using very large numbers with udelay(), use mdelay(), which
is easier to follow.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index f36bec8b3a8a..6f601bcf8646 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -71,9 +71,7 @@ static void ide_reset(void)
 	/* de-assert RESET signal */
 	ide_set_reset(0);
 
-	/* wait 250 ms */
-	for (i = 0; i < 250; ++i)
-		udelay(1000);
+	mdelay(250);
 }
 #else
 #define ide_reset()	/* dummy */
@@ -237,7 +235,7 @@ unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 	ide_output_data_shorts(device, (unsigned short *)ccb, ccblen / 2);
 
 	/* ATAPI Command written wait for completition */
-	udelay(5000);		/* device must set bsy */
+	mdelay(5);		/* device must set bsy */
 
 	mask = ATA_STAT_DRQ | ATA_STAT_BUSY | ATA_STAT_ERR;
 	/*
@@ -293,7 +291,7 @@ unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 					      n);
 		}
 	}
-	udelay(5000);		/* seems that some CD ROMs need this... */
+	mdelay(5);		/* seems that some CD ROMs need this... */
 	mask = ATA_STAT_BUSY | ATA_STAT_ERR;
 	res = 0;
 	c = atapi_wait_mask(device, ATAPI_TIME_OUT, mask, res);
@@ -357,7 +355,7 @@ retry:
 
 	if ((key == 6) || (asc == 0x29) || (asc == 0x28)) { /* Unit Attention */
 		if (unitattn-- > 0) {
-			udelay(200 * 1000);
+			mdelay(200);
 			goto retry;
 		}
 		printf("Unit Attention, tried %d\n", ATAPI_UNIT_ATTN);
@@ -366,7 +364,7 @@ retry:
 	if ((asc == 0x4) && (ascq == 0x1)) {
 		/* not ready, but will be ready soon */
 		if (notready-- > 0) {
-			udelay(200 * 1000);
+			mdelay(200);
 			goto retry;
 		}
 		printf("Drive not ready, tried %d times\n",
@@ -586,9 +584,9 @@ static void ide_ident(struct blk_desc *dev_desc)
 				debug("Retrying...\n");
 				ide_outb(device, ATA_DEV_HD,
 					 ATA_LBA | ATA_DEVICE(device));
-				udelay(100000);
+				mdelay(100);
 				ide_outb(device, ATA_COMMAND, 0x08);
-				udelay(500000);	/* 500 ms */
+				mdelay(500);
 			}
 			/*
 			 * Select device
@@ -715,14 +713,13 @@ void ide_init(void)
 
 		ide_bus_ok[bus] = 0;
 
-		/* Select device
-		 */
-		udelay(100000);	/* 100 ms */
+		/* Select device */
+		mdelay(100);
 		ide_outb(dev, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(dev));
-		udelay(100000);	/* 100 ms */
+		mdelay(100);
 		i = 0;
 		do {
-			udelay(10000);	/* 10 ms */
+			mdelay(10);
 
 			c = ide_inb(dev, ATA_STATUS);
 			i++;
-- 
2.40.0.348.gf938b09366-goog

[PATCH 03/30] ide: Drop CONFIG_START_IDE

[Simon Glass]

This is not used by any board. Drop it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/board_r.c | 5 -----
 include/ide.h    | 7 -------
 2 files changed, 12 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index 6b4180b3ecde..7076af64f5de 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -523,12 +523,7 @@ static int initr_post(void)
 static int initr_ide(void)
 {
 	puts("IDE:   ");
-#if defined(CONFIG_START_IDE)
-	if (board_start_ide())
-		ide_init();
-#else
 	ide_init();
-#endif
 	return 0;
 }
 #endif
diff --git a/include/ide.h b/include/ide.h
index 58f6640c61be..9c0d40364a8f 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -47,11 +47,4 @@ void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts);
 
 void ide_led(uchar led, uchar status);
 
-/**
- * board_start_ide() - Start up the board IDE interfac
- *
- * Return: 0 if ok
- */
-int board_start_ide(void);
-
 #endif /* _IDE_H */
-- 
2.40.0.348.gf938b09366-goog

[PATCH 04/30] ide: Drop init for not using BLK

[Simon Glass]

ALl boards use CONFIG_BLK now so this code is not used. Drop it and the
header-file #ifdef

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/board_r.c | 12 ------------
 include/ide.h    |  7 -------
 2 files changed, 19 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index 7076af64f5de..d798c00a80a5 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -519,15 +519,6 @@ static int initr_post(void)
 }
 #endif
 
-#if defined(CONFIG_IDE) && !defined(CONFIG_BLK)
-static int initr_ide(void)
-{
-	puts("IDE:   ");
-	ide_init();
-	return 0;
-}
-#endif
-
 #if defined(CFG_PRAM)
 /*
  * Export available size of memory for Linux, taking into account the
@@ -778,9 +769,6 @@ static init_fnc_t init_sequence_r[] = {
 #ifdef CONFIG_POST
 	initr_post,
 #endif
-#if defined(CONFIG_IDE) && !defined(CONFIG_BLK)
-	initr_ide,
-#endif
 #ifdef CONFIG_LAST_STAGE_INIT
 	INIT_FUNC_WATCHDOG_RESET
 	/*
diff --git a/include/ide.h b/include/ide.h
index 9c0d40364a8f..457f275c61b4 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -18,17 +18,10 @@
 void ide_init(void);
 struct blk_desc;
 struct udevice;
-#ifdef CONFIG_BLK
 ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 	       void *buffer);
 ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 		const void *buffer);
-#else
-ulong ide_read(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt,
-	       void *buffer);
-ulong ide_write(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt,
-		const void *buffer);
-#endif
 
 #if defined(CONFIG_OF_IDE_FIXUP)
 int ide_device_present(int dev);
-- 
2.40.0.348.gf938b09366-goog

[PATCH 05/30] ide: Move ide_init() into probing

[Simon Glass]

At present the code does ide_init() as a separate operation, then calls
device_probe() to copy over the information. We can call ide_init() from
probe just as easily.

The only difference is that using 'ide init' twice will do nothing.
However it already fails to copy over the new data in that case, so the
effect is the same. For now, unbind the block devices and remove the IDE
device, which causes the bus to be probed again. Later patches will fix
this up fully, so that all blk_desc data is copied across.

Since ide_reset() is only called from ide_init(), there is no need to init
the ide_dev_desc[] array. This is already done at the end of ide_init() so
drop this code.

The call to uclass_first_device() is now within the probe() function of
the same device, so does nothing. Drop it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 cmd/ide.c           | 22 +++++++++++++++++++++-
 drivers/block/ide.c | 13 ++++++-------
 include/ide.h       |  1 -
 test/boot/bootdev.c |  2 --
 4 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/cmd/ide.c b/cmd/ide.c
index 6739f0b12d1a..ddc87d3a0bb9 100644
--- a/cmd/ide.c
+++ b/cmd/ide.c
@@ -10,12 +10,15 @@
 
 #include <common.h>
 #include <blk.h>
+#include <dm.h>
 #include <config.h>
 #include <watchdog.h>
 #include <command.h>
 #include <image.h>
 #include <asm/byteorder.h>
 #include <asm/io.h>
+#include <dm/device-internal.h>
+#include <dm/uclass-internal.h>
 
 #include <ide.h>
 #include <ata.h>
@@ -31,8 +34,25 @@ int do_ide(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 {
 	if (argc == 2) {
 		if (strncmp(argv[1], "res", 3) == 0) {
+			struct udevice *dev;
+			int ret;
+
 			puts("\nReset IDE: ");
-			ide_init();
+			ret = uclass_find_first_device(UCLASS_IDE, &dev);
+			ret = device_remove(dev, DM_REMOVE_NORMAL);
+			if (!ret)
+				ret = device_chld_unbind(dev, NULL);
+			if (ret) {
+				printf("Cannot remove IDE (err=%dE)\n", ret);
+				return CMD_RET_FAILURE;
+			}
+
+			ret = uclass_first_device_err(UCLASS_IDE, &dev);
+			if (ret) {
+				printf("Init failed (err=%dE)\n", ret);
+				return CMD_RET_FAILURE;
+			}
+
 			return 0;
 		}
 	}
diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 6f601bcf8646..13770484b36a 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -58,8 +58,6 @@ static void ide_reset(void)
 
 	for (i = 0; i < CONFIG_SYS_IDE_MAXBUS; ++i)
 		ide_bus_ok[i] = 0;
-	for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; ++i)
-		ide_dev_desc[i].type = DEV_TYPE_UNKNOWN;
 
 	ide_set_reset(1);	/* assert reset */
 
@@ -689,9 +687,8 @@ __weak unsigned char ide_inb(int dev, int port)
 	return val;
 }
 
-void ide_init(void)
+static void ide_init(void)
 {
-	struct udevice *dev;
 	unsigned char c;
 	int i, bus;
 
@@ -764,8 +761,6 @@ void ide_init(void)
 		dev_print(&ide_dev_desc[i]);
 	}
 	schedule();
-
-	uclass_first_device(UCLASS_IDE, &dev);
 }
 
 __weak void ide_input_swap_data(int dev, ulong *sect_buf, int words)
@@ -1067,7 +1062,9 @@ static int ide_bootdev_bind(struct udevice *dev)
 
 static int ide_bootdev_hunt(struct bootdev_hunter *info, bool show)
 {
-	ide_init();
+	struct udevice *dev;
+
+	uclass_first_device(UCLASS_IDE, &dev);
 
 	return 0;
 }
@@ -1104,6 +1101,8 @@ static int ide_probe(struct udevice *udev)
 	int i;
 	int ret;
 
+	ide_init();
+
 	for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
 		if (ide_dev_desc[i].type != DEV_TYPE_UNKNOWN) {
 			sprintf(name, "blk#%d", i);
diff --git a/include/ide.h b/include/ide.h
index 457f275c61b4..3f36b4340d06 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -15,7 +15,6 @@
  * Function Prototypes
  */
 
-void ide_init(void);
 struct blk_desc;
 struct udevice;
 ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c
index 4fe9fd722084..365b7f518ab2 100644
--- a/test/boot/bootdev.c
+++ b/test/boot/bootdev.c
@@ -376,7 +376,6 @@ static int bootdev_test_cmd_hunt(struct unit_test_state *uts)
 	ut_assert_nextline("Hunting with: simple_bus");
 	ut_assert_nextline("Found 2 extension board(s).");
 	ut_assert_nextline("Hunting with: ide");
-	ut_assert_nextline("Bus 0: not available  ");
 
 	/* mmc hunter has already been used so should not run again */
 
@@ -487,7 +486,6 @@ static int bootdev_test_hunt_prio(struct unit_test_state *uts)
 	/* now try a different priority, verbosely */
 	ut_assertok(bootdev_hunt_prio(BOOTDEVP_5_SCAN_SLOW, true));
 	ut_assert_nextline("Hunting with: ide");
-	ut_assert_nextline("Bus 0: not available  ");
 	ut_assert_nextline("Hunting with: usb");
 	ut_assert_nextline(
 		"Bus usb@1: scanning bus usb@1 for devices... 5 USB Device(s) found");
-- 
2.40.0.348.gf938b09366-goog

[PATCH 06/30] ide: Drop ide_device_present()

[Simon Glass]

This function is not used anymore. Drop it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 9 ---------
 include/ide.h       | 4 ----
 2 files changed, 13 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 13770484b36a..b5be022a0673 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -1012,15 +1012,6 @@ WR_OUT:
 	return n;
 }
 
-#if defined(CONFIG_OF_IDE_FIXUP)
-int ide_device_present(int dev)
-{
-	if (dev >= CONFIG_SYS_IDE_MAXBUS)
-		return 0;
-	return ide_dev_desc[dev].type == DEV_TYPE_UNKNOWN ? 0 : 1;
-}
-#endif
-
 static int ide_blk_probe(struct udevice *udev)
 {
 	struct blk_desc *desc = dev_get_uclass_plat(udev);
diff --git a/include/ide.h b/include/ide.h
index 3f36b4340d06..09b0117879f7 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -22,10 +22,6 @@ ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 		const void *buffer);
 
-#if defined(CONFIG_OF_IDE_FIXUP)
-int ide_device_present(int dev);
-#endif
-
 /*
  * I/O function overrides
  */
-- 
2.40.0.348.gf938b09366-goog

[PATCH 08/30] ide: Drop weak functions

[Simon Glass]

These are not used from outside this file anymore. Make them static and
remove them from the header file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 14 +++++++-------
 include/ide.h       | 13 -------------
 2 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 58f1ef8f1777..46e110fec5e8 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -75,7 +75,7 @@ static void ide_reset(void)
 #define ide_reset()	/* dummy */
 #endif /* CONFIG_IDE_RESET */
 
-__weak void ide_outb(int dev, int port, unsigned char val)
+static void ide_outb(int dev, int port, unsigned char val)
 {
 	debug("ide_outb (dev= %d, port= 0x%x, val= 0x%02x) : @ 0x%08lx\n",
 	      dev, port, val, ATA_CURR_BASE(dev) + port);
@@ -83,7 +83,7 @@ __weak void ide_outb(int dev, int port, unsigned char val)
 	outb(val, ATA_CURR_BASE(dev) + port);
 }
 
-__weak unsigned char ide_inb(int dev, int port)
+static unsigned char ide_inb(int dev, int port)
 {
 	uchar val;
 
@@ -94,7 +94,7 @@ __weak unsigned char ide_inb(int dev, int port)
 	return val;
 }
 
-__weak void ide_input_swap_data(int dev, ulong *sect_buf, int words)
+static void ide_input_swap_data(int dev, ulong *sect_buf, int words)
 {
 	uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
 	ushort *dbuf = (ushort *)sect_buf;
@@ -164,7 +164,7 @@ OUT:
 
 /* since ATAPI may use commands with not 4 bytes alligned length
  * we have our own transfer functions, 2 bytes alligned */
-__weak void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts)
+static void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts)
 {
 	uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
 	ushort *dbuf;
@@ -179,7 +179,7 @@ __weak void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts)
 	}
 }
 
-__weak void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts)
+static void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts)
 {
 	uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
 	ushort *dbuf;
@@ -778,7 +778,7 @@ static void ide_init(void)
 	schedule();
 }
 
-__weak void ide_output_data(int dev, const ulong *sect_buf, int words)
+static void ide_output_data(int dev, const ulong *sect_buf, int words)
 {
 	uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
 	ushort *dbuf;
@@ -792,7 +792,7 @@ __weak void ide_output_data(int dev, const ulong *sect_buf, int words)
 	}
 }
 
-__weak void ide_input_data(int dev, ulong *sect_buf, int words)
+static void ide_input_data(int dev, ulong *sect_buf, int words)
 {
 	uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
 	ushort *dbuf;
diff --git a/include/ide.h b/include/ide.h
index 09b0117879f7..8c0eb2a022fd 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -22,17 +22,4 @@ ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 		const void *buffer);
 
-/*
- * I/O function overrides
- */
-unsigned char ide_inb(int dev, int port);
-void ide_outb(int dev, int port, unsigned char val);
-void ide_input_swap_data(int dev, ulong *sect_buf, int words);
-void ide_input_data(int dev, ulong *sect_buf, int words);
-void ide_output_data(int dev, const ulong *sect_buf, int words);
-void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts);
-void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts);
-
-void ide_led(uchar led, uchar status);
-
 #endif /* _IDE_H */
-- 
2.40.0.348.gf938b09366-goog

[PATCH 09/30] ide: Create a prototype for ide_set_reset()

[Simon Glass]

This is used by a board so should be in the header file. Add it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c |  2 --
 include/ide.h       | 10 ++++++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 46e110fec5e8..fa5f68ffeb01 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -50,8 +50,6 @@ struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
 #define IDE_SPIN_UP_TIME_OUT 5000 /* 5 sec spin-up timeout */
 
 #ifdef CONFIG_IDE_RESET
-extern void ide_set_reset(int idereset);
-
 static void ide_reset(void)
 {
 	int i;
diff --git a/include/ide.h b/include/ide.h
index 8c0eb2a022fd..b586ba3df4bf 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -22,4 +22,14 @@ ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 		const void *buffer);
 
+/**
+ * ide_set_reset() - Assert or de-assert reset for the IDE device
+ *
+ * This is provided by boards which need to reset the device through another
+ * means, e.g. a GPIO.
+ *
+ * @idereset: 1 to assert reset, 0 to de-assert it
+ */
+void ide_set_reset(int idereset);
+
 #endif /* _IDE_H */
-- 
2.40.0.348.gf938b09366-goog

[PATCH 10/30] ide: Correct use of ATAPI

[Simon Glass]

The use of atapi_read() was incorrect dropped. Fix this so that it will
be used when needed. Use a udevice for the first argument of atapi_read()
so it is consistent with ide_read().

This requires much of the ATAPI code to be brought out from behind the
existing #ifdef. It will still be removed by the compiler if it is not
needed.

Add an atapi flag to struct blk_desc so the information can be retained.

Fixes: 145df842b44 ("dm: ide: Add support for driver-model block devices")
Fixes: d0075059e4d ("ide: Drop non-DM code for BLK")

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 20 +++++++++++++++++---
 include/blk.h       |  1 +
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index fa5f68ffeb01..875192cba163 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -155,7 +155,6 @@ OUT:
 	*last = '\0';
 }
 
-#ifdef CONFIG_ATAPI
 /****************************************************************************
  * ATAPI Support
  */
@@ -422,9 +421,10 @@ error:
 #define ATAPI_READ_BLOCK_SIZE	2048	/* assuming CD part */
 #define ATAPI_READ_MAX_BLOCK	(ATAPI_READ_MAX_BYTES/ATAPI_READ_BLOCK_SIZE)
 
-ulong atapi_read(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt,
+ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 		 void *buffer)
 {
+	struct blk_desc *block_dev = dev_get_uclass_plat(dev);
 	int device = block_dev->devnum;
 	ulong n = 0;
 	unsigned char ccb[12];	/* Command descriptor block */
@@ -466,6 +466,8 @@ ulong atapi_read(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt,
 	return n;
 }
 
+#ifdef CONFIG_ATAPI
+
 static void atapi_inquiry(struct blk_desc *dev_desc)
 {
 	unsigned char ccb[12];	/* Command descriptor block */
@@ -653,6 +655,7 @@ static void ide_ident(struct blk_desc *dev_desc)
 
 #ifdef CONFIG_ATAPI
 	if (is_atapi) {
+		dev_desc->atapi = true;
 		atapi_inquiry(dev_desc);
 		return;
 	}
@@ -1010,6 +1013,17 @@ WR_OUT:
 	return n;
 }
 
+ulong ide_or_atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
+			void *buffer)
+{
+	struct blk_desc *desc = dev_get_uclass_plat(dev);
+
+	if (IS_ENABLED(CONFIG_ATAPI) && desc->atapi)
+		return atapi_read(dev, blknr, blkcnt, buffer);
+
+	return ide_read(dev, blknr, blkcnt, buffer);
+}
+
 static int ide_blk_probe(struct udevice *udev)
 {
 	struct blk_desc *desc = dev_get_uclass_plat(udev);
@@ -1029,7 +1043,7 @@ static int ide_blk_probe(struct udevice *udev)
 }
 
 static const struct blk_ops ide_blk_ops = {
-	.read	= ide_read,
+	.read	= ide_or_atapi_read,
 	.write	= ide_write,
 };
 
diff --git a/include/blk.h b/include/blk.h
index 1db203c1baba..871922dcde07 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -66,6 +66,7 @@ struct blk_desc {
 	/* device can use 48bit addr (ATA/ATAPI v7) */
 	unsigned char	lba48;
 #endif
+	unsigned char	atapi;		/* Use ATAPI protocol */
 	lbaint_t	lba;		/* number of blocks */
 	unsigned long	blksz;		/* block size */
 	int		log2blksz;	/* for convenience: log2(blksz) */
-- 
2.40.0.348.gf938b09366-goog

[PATCH 11/30] ide: Make function static

[Simon Glass]

Only one function is called from outside this file. Make all the others
static.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 23 +++++++++++------------
 include/ide.h       | 11 -----------
 2 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 875192cba163..1d5e54d6eb98 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -219,8 +219,8 @@ static uchar atapi_wait_mask(int dev, ulong t, uchar mask, uchar res)
 /*
  * issue an atapi command
  */
-unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
-			  unsigned char *buffer, int buflen)
+static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
+				 unsigned char *buffer, int buflen)
 {
 	unsigned char c, err, mask, res;
 	int n;
@@ -343,10 +343,9 @@ AI_OUT:
 #define ATAPI_DRIVE_NOT_READY	100
 #define ATAPI_UNIT_ATTN		10
 
-unsigned char atapi_issue_autoreq(int device,
-				  unsigned char *ccb,
-				  int ccblen,
-				  unsigned char *buffer, int buflen)
+static unsigned char atapi_issue_autoreq(int device, unsigned char *ccb,
+					 int ccblen,
+					 unsigned char *buffer, int buflen)
 {
 	unsigned char sense_data[18], sense_ccb[12];
 	unsigned char res, key, asc, ascq;
@@ -421,8 +420,8 @@ error:
 #define ATAPI_READ_BLOCK_SIZE	2048	/* assuming CD part */
 #define ATAPI_READ_MAX_BLOCK	(ATAPI_READ_MAX_BYTES/ATAPI_READ_BLOCK_SIZE)
 
-ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
-		 void *buffer)
+static ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
+			void *buffer)
 {
 	struct blk_desc *block_dev = dev_get_uclass_plat(dev);
 	int device = block_dev->devnum;
@@ -810,8 +809,8 @@ static void ide_input_data(int dev, ulong *sect_buf, int words)
 	}
 }
 
-ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
-	       void *buffer)
+static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
+		      void *buffer)
 {
 	struct blk_desc *block_dev = dev_get_uclass_plat(dev);
 	int device = block_dev->devnum;
@@ -930,8 +929,8 @@ IDE_READ_E:
 	return n;
 }
 
-ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
-		const void *buffer)
+static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
+		       const void *buffer)
 {
 	struct blk_desc *block_dev = dev_get_uclass_plat(dev);
 	int device = block_dev->devnum;
diff --git a/include/ide.h b/include/ide.h
index b586ba3df4bf..2c25e74ede08 100644
--- a/include/ide.h
+++ b/include/ide.h
@@ -11,17 +11,6 @@
 
 #define IDE_BUS(dev)	(dev / (CONFIG_SYS_IDE_MAXDEVICE / CONFIG_SYS_IDE_MAXBUS))
 
-/*
- * Function Prototypes
- */
-
-struct blk_desc;
-struct udevice;
-ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
-	       void *buffer);
-ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
-		const void *buffer);
-
 /**
  * ide_set_reset() - Assert or de-assert reset for the IDE device
  *
-- 
2.40.0.348.gf938b09366-goog

[PATCH 12/30] ide: Change the retries variable

[Simon Glass]

Use a 'tries' variable which starts at the number of tries we want to do,
rather than a 'retries' one that stops at either 1 or 2. This will make it
easier to refactor the code to avoid the horrible #ifdefs

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 1d5e54d6eb98..782780fd302b 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -557,7 +557,7 @@ static void ide_ident(struct blk_desc *dev_desc)
 	hd_driveid_t iop;
 #ifdef CONFIG_ATAPI
 	bool is_atapi = false;
-	int retries = 0;
+	int tries = 1;
 #endif
 	int device;
 
@@ -570,10 +570,10 @@ static void ide_ident(struct blk_desc *dev_desc)
 	dev_desc->uclass_id = UCLASS_IDE;
 #ifdef CONFIG_ATAPI
 
-	retries = 0;
+	tries = 2;
 
 	/* Warning: This will be tricky to read */
-	while (retries <= 1) {
+	while (tries) {
 		/* check signature */
 		if ((ide_inb(device, ATA_SECT_CNT) == 0x01) &&
 		    (ide_inb(device, ATA_SECT_NUM) == 0x01) &&
@@ -624,7 +624,7 @@ static void ide_ident(struct blk_desc *dev_desc)
 			 */
 			ide_outb(device, ATA_DEV_HD,
 				 ATA_LBA | ATA_DEVICE(device));
-			retries++;
+			tries--;
 #else
 			return;
 #endif
@@ -634,7 +634,7 @@ static void ide_ident(struct blk_desc *dev_desc)
 			break;
 	}			/* see above - ugly to read */
 
-	if (retries == 2)	/* Not found */
+	if (!tries)	/* Not found */
 		return;
 #endif
 
-- 
2.40.0.348.gf938b09366-goog

[PATCH 13/30] ide: Refactor confusing loop code

[Simon Glass]

This code is hard to follow as it uses #ifdef in a strange way. Adjust
it to avoid the preprocessor. Drop the special return for the non-ATAPI
case since we can rely on tries becoming 0 and exiting the loop.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 43 +++++++++++++++----------------------------
 1 file changed, 15 insertions(+), 28 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 782780fd302b..2f45bb6bffb2 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -555,10 +555,8 @@ static void ide_ident(struct blk_desc *dev_desc)
 {
 	unsigned char c;
 	hd_driveid_t iop;
-#ifdef CONFIG_ATAPI
 	bool is_atapi = false;
 	int tries = 1;
-#endif
 	int device;
 
 	device = dev_desc->devnum;
@@ -568,17 +566,16 @@ static void ide_ident(struct blk_desc *dev_desc)
 	 */
 	ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
 	dev_desc->uclass_id = UCLASS_IDE;
-#ifdef CONFIG_ATAPI
+	if (IS_ENABLED(CONFIG_ATAPI))
+		tries = 2;
 
-	tries = 2;
-
-	/* Warning: This will be tricky to read */
 	while (tries) {
 		/* check signature */
-		if ((ide_inb(device, ATA_SECT_CNT) == 0x01) &&
-		    (ide_inb(device, ATA_SECT_NUM) == 0x01) &&
-		    (ide_inb(device, ATA_CYL_LOW) == 0x14) &&
-		    (ide_inb(device, ATA_CYL_HIGH) == 0xEB)) {
+		if (IS_ENABLED(CONFIG_ATAPI) &&
+		    ide_inb(device, ATA_SECT_CNT) == 0x01 &&
+		    ide_inb(device, ATA_SECT_NUM) == 0x01 &&
+		    ide_inb(device, ATA_CYL_LOW) == 0x14 &&
+		    ide_inb(device, ATA_CYL_HIGH) == 0xeb) {
 			/* ATAPI Signature found */
 			is_atapi = true;
 			/*
@@ -590,9 +587,7 @@ static void ide_ident(struct blk_desc *dev_desc)
 			 * to become ready
 			 */
 			c = ide_wait(device, ATAPI_TIME_OUT);
-		} else
-#endif
-		{
+		} else {
 			/*
 			 * Start Ident Command
 			 */
@@ -606,8 +601,7 @@ static void ide_ident(struct blk_desc *dev_desc)
 
 		if (((c & ATA_STAT_DRQ) == 0) ||
 		    ((c & (ATA_STAT_FAULT | ATA_STAT_ERR)) != 0)) {
-#ifdef CONFIG_ATAPI
-			{
+			if (IS_ENABLED(CONFIG_ATAPI)) {
 				/*
 				 * Need to soft reset the device
 				 * in case it's an ATAPI...
@@ -618,25 +612,18 @@ static void ide_ident(struct blk_desc *dev_desc)
 				mdelay(100);
 				ide_outb(device, ATA_COMMAND, 0x08);
 				mdelay(500);
+				/* Select device */
+				ide_outb(device, ATA_DEV_HD,
+					 ATA_LBA | ATA_DEVICE(device));
 			}
-			/*
-			 * Select device
-			 */
-			ide_outb(device, ATA_DEV_HD,
-				 ATA_LBA | ATA_DEVICE(device));
 			tries--;
-#else
-			return;
-#endif
-		}
-#ifdef CONFIG_ATAPI
-		else
+		} else {
 			break;
-	}			/* see above - ugly to read */
+		}
+	}
 
 	if (!tries)	/* Not found */
 		return;
-#endif
 
 	ide_input_swap_data(device, (ulong *)&iop, ATA_SECTORWORDS);
 
-- 
2.40.0.348.gf938b09366-goog

[PATCH 14/30] ide: Simplify success condition

[Simon Glass]

Change the if() to remove extra brackets and check for the positive case
first, i.e. when a device is found. Exit the loop in that case, with the
retry logic in the 'else' part.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 2f45bb6bffb2..a51a0008cae4 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -599,27 +599,25 @@ static void ide_ident(struct blk_desc *dev_desc)
 			c = ide_wait(device, IDE_TIME_OUT);
 		}
 
-		if (((c & ATA_STAT_DRQ) == 0) ||
-		    ((c & (ATA_STAT_FAULT | ATA_STAT_ERR)) != 0)) {
-			if (IS_ENABLED(CONFIG_ATAPI)) {
-				/*
-				 * Need to soft reset the device
-				 * in case it's an ATAPI...
-				 */
-				debug("Retrying...\n");
-				ide_outb(device, ATA_DEV_HD,
-					 ATA_LBA | ATA_DEVICE(device));
-				mdelay(100);
-				ide_outb(device, ATA_COMMAND, 0x08);
-				mdelay(500);
-				/* Select device */
-				ide_outb(device, ATA_DEV_HD,
-					 ATA_LBA | ATA_DEVICE(device));
-			}
-			tries--;
-		} else {
+		if ((c & ATA_STAT_DRQ) &&
+		    !(c & (ATA_STAT_FAULT | ATA_STAT_ERR))) {
 			break;
+		} else if (IS_ENABLED(CONFIG_ATAPI)) {
+			/*
+			 * Need to soft reset the device
+			 * in case it's an ATAPI...
+			 */
+			debug("Retrying...\n");
+			ide_outb(device, ATA_DEV_HD,
+				 ATA_LBA | ATA_DEVICE(device));
+			mdelay(100);
+			ide_outb(device, ATA_COMMAND, 0x08);
+			mdelay(500);
+			/* Select device */
+			ide_outb(device, ATA_DEV_HD,
+				 ATA_LBA | ATA_DEVICE(device));
 		}
+		tries--;
 	}
 
 	if (!tries)	/* Not found */
-- 
2.40.0.348.gf938b09366-goog

[PATCH 15/30] ide: Avoid preprocessor for CONFIG_ATAPI

[Simon Glass]

Use IS_ENABLED() instead for all conditions.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index a51a0008cae4..6c5227a5c0e2 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -465,8 +465,6 @@ static ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 	return n;
 }
 
-#ifdef CONFIG_ATAPI
-
 static void atapi_inquiry(struct blk_desc *dev_desc)
 {
 	unsigned char ccb[12];	/* Command descriptor block */
@@ -549,8 +547,6 @@ static void atapi_inquiry(struct blk_desc *dev_desc)
 	return;
 }
 
-#endif /* CONFIG_ATAPI */
-
 static void ide_ident(struct blk_desc *dev_desc)
 {
 	unsigned char c;
@@ -637,13 +633,11 @@ static void ide_ident(struct blk_desc *dev_desc)
 	else
 		dev_desc->removable = 0;
 
-#ifdef CONFIG_ATAPI
-	if (is_atapi) {
+	if (IS_ENABLED(CONFIG_ATAPI) && is_atapi) {
 		dev_desc->atapi = true;
 		atapi_inquiry(dev_desc);
 		return;
 	}
-#endif /* CONFIG_ATAPI */
 
 	iop.lba_capacity[0] = be16_to_cpu(iop.lba_capacity[0]);
 	iop.lba_capacity[1] = be16_to_cpu(iop.lba_capacity[1]);
@@ -732,11 +726,10 @@ static void ide_init(void)
 		if (c & (ATA_STAT_BUSY | ATA_STAT_FAULT)) {
 			puts("not available  ");
 			debug("Status = 0x%02X ", c);
-#ifndef CONFIG_ATAPI		/* ATAPI Devices do not set DRDY */
-		} else if ((c & ATA_STAT_READY) == 0) {
+		} else if (IS_ENABLED(CONFIG_ATAPI) && !(c & ATA_STAT_READY)) {
+			/* ATAPI Devices do not set DRDY */
 			puts("not available  ");
 			debug("Status = 0x%02X ", c);
-#endif
 		} else {
 			puts("OK ");
 			ide_bus_ok[bus] = 1;
-- 
2.40.0.348.gf938b09366-goog

[PATCH 16/30] ide: Avoid preprocessor for CONFIG_LBA48

[Simon Glass]

Use IS_ENABLED() instead for all conditions. Add the 'lba48' flag into
struct blk_desc always, since it uses very little space. Use a bool so
the meaning is clearer.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 57 ++++++++++++++++-----------------------------
 include/blk.h       |  4 +---
 2 files changed, 21 insertions(+), 40 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 6c5227a5c0e2..45201333c3c5 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -540,11 +540,9 @@ static void atapi_inquiry(struct blk_desc *dev_desc)
 		((unsigned long) iobuf[5] << 16) +
 		((unsigned long) iobuf[6] << 8) + ((unsigned long) iobuf[7]);
 	dev_desc->log2blksz = LOG2(dev_desc->blksz);
-#ifdef CONFIG_LBA48
+
 	/* ATAPI devices cannot use 48bit addressing (ATA/ATAPI v7) */
-	dev_desc->lba48 = 0;
-#endif
-	return;
+	dev_desc->lba48 = false;
 }
 
 static void ide_ident(struct blk_desc *dev_desc)
@@ -645,9 +643,9 @@ static void ide_ident(struct blk_desc *dev_desc)
 			((unsigned long)iop.lba_capacity[0]) |
 			((unsigned long)iop.lba_capacity[1] << 16);
 
-#ifdef CONFIG_LBA48
-	if (iop.command_set_2 & 0x0400) {	/* LBA 48 support */
-		dev_desc->lba48 = 1;
+	if (IS_ENABLED(CONFIG_LBA48) && (iop.command_set_2 & 0x0400)) {
+		/* LBA 48 support */
+		dev_desc->lba48 = true;
 		for (int i = 0; i < 4; i++)
 			iop.lba48_capacity[i] = be16_to_cpu(iop.lba48_capacity[i]);
 		dev_desc->lba =
@@ -656,9 +654,9 @@ static void ide_ident(struct blk_desc *dev_desc)
 			((unsigned long long)iop.lba48_capacity[2] << 32) |
 			((unsigned long long)iop.lba48_capacity[3] << 48));
 	} else {
-		dev_desc->lba48 = 0;
+		dev_desc->lba48 = false;
 	}
-#endif /* CONFIG_LBA48 */
+
 	/* assuming HD */
 	dev_desc->type = DEV_TYPE_HARDDISK;
 	dev_desc->blksz = ATA_BLOCKSIZE;
@@ -795,15 +793,13 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 	ulong n = 0;
 	unsigned char c;
 	unsigned char pwrsave = 0;	/* power save */
+	bool lba48 = false;
 
-#ifdef CONFIG_LBA48
-	unsigned char lba48 = 0;
-
-	if (blknr & 0x0000fffff0000000ULL) {
+	if (IS_ENABLED(CONFIG_LBA48) && (blknr & 0x0000fffff0000000ULL)) {
 		/* more than 28 bits used, use 48bit mode */
-		lba48 = 1;
+		lba48 = true;
 	}
-#endif
+
 	debug("ide_read dev %d start " LBAF ", blocks " LBAF " buffer at %lX\n",
 	      device, blknr, blkcnt, (ulong) buffer);
 
@@ -845,8 +841,7 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 			printf("IDE read: device %d not ready\n", device);
 			break;
 		}
-#ifdef CONFIG_LBA48
-		if (lba48) {
+		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
 			/* write high bits */
 			ide_outb(device, ATA_SECT_CNT, 0);
 			ide_outb(device, ATA_LBA_LOW, (blknr >> 24) & 0xFF);
@@ -858,21 +853,17 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 			ide_outb(device, ATA_LBA_HIGH, 0);
 #endif
 		}
-#endif
 		ide_outb(device, ATA_SECT_CNT, 1);
 		ide_outb(device, ATA_LBA_LOW, (blknr >> 0) & 0xFF);
 		ide_outb(device, ATA_LBA_MID, (blknr >> 8) & 0xFF);
 		ide_outb(device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF);
 
-#ifdef CONFIG_LBA48
-		if (lba48) {
+		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
 			ide_outb(device, ATA_DEV_HD,
 				 ATA_LBA | ATA_DEVICE(device));
 			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ_EXT);
 
-		} else
-#endif
-		{
+		} else {
 			ide_outb(device, ATA_DEV_HD, ATA_LBA |
 				 ATA_DEVICE(device) | ((blknr >> 24) & 0xF));
 			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ);
@@ -914,15 +905,12 @@ static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 	int device = block_dev->devnum;
 	ulong n = 0;
 	unsigned char c;
+	bool lba48 = false;
 
-#ifdef CONFIG_LBA48
-	unsigned char lba48 = 0;
-
-	if (blknr & 0x0000fffff0000000ULL) {
+	if (IS_ENABLED(CONFIG_LBA48) && (blknr & 0x0000fffff0000000ULL)) {
 		/* more than 28 bits used, use 48bit mode */
-		lba48 = 1;
+		lba48 = true;
 	}
-#endif
 
 	/* Select device
 	 */
@@ -935,8 +923,7 @@ static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 			printf("IDE read: device %d not ready\n", device);
 			goto WR_OUT;
 		}
-#ifdef CONFIG_LBA48
-		if (lba48) {
+		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
 			/* write high bits */
 			ide_outb(device, ATA_SECT_CNT, 0);
 			ide_outb(device, ATA_LBA_LOW, (blknr >> 24) & 0xFF);
@@ -948,21 +935,17 @@ static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 			ide_outb(device, ATA_LBA_HIGH, 0);
 #endif
 		}
-#endif
 		ide_outb(device, ATA_SECT_CNT, 1);
 		ide_outb(device, ATA_LBA_LOW, (blknr >> 0) & 0xFF);
 		ide_outb(device, ATA_LBA_MID, (blknr >> 8) & 0xFF);
 		ide_outb(device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF);
 
-#ifdef CONFIG_LBA48
-		if (lba48) {
+		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
 			ide_outb(device, ATA_DEV_HD,
 				 ATA_LBA | ATA_DEVICE(device));
 			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_WRITE_EXT);
 
-		} else
-#endif
-		{
+		} else {
 			ide_outb(device, ATA_DEV_HD, ATA_LBA |
 				 ATA_DEVICE(device) | ((blknr >> 24) & 0xF));
 			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_WRITE);
diff --git a/include/blk.h b/include/blk.h
index 871922dcde07..2c9c7985a885 100644
--- a/include/blk.h
+++ b/include/blk.h
@@ -62,10 +62,8 @@ struct blk_desc {
 	unsigned char	hwpart;		/* HW partition, e.g. for eMMC */
 	unsigned char	type;		/* device type */
 	unsigned char	removable;	/* removable device */
-#ifdef CONFIG_LBA48
 	/* device can use 48bit addr (ATA/ATAPI v7) */
-	unsigned char	lba48;
-#endif
+	bool	lba48;
 	unsigned char	atapi;		/* Use ATAPI protocol */
 	lbaint_t	lba;		/* number of blocks */
 	unsigned long	blksz;		/* block size */
-- 
2.40.0.348.gf938b09366-goog

[PATCH 17/30] ide: Move bus init into a function

[Simon Glass]

Move this code into a separate function which returns whether the bus was
found, or not.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 87 +++++++++++++++++++++++++--------------------
 1 file changed, 48 insertions(+), 39 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 45201333c3c5..aac4462f57b9 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -679,9 +679,55 @@ static void ide_ident(struct blk_desc *dev_desc)
 #endif
 }
 
+/**
+ * ide_init_one() - Init one IDE device
+ *
+ * @bus: Bus to use
+ * Return: 0 iuf OK, -EIO if not available, -ETIMEDOUT if timed out
+ */
+static int ide_init_one(int bus)
+{
+	int dev = bus * CONFIG_SYS_IDE_MAXDEVICE / CONFIG_SYS_IDE_MAXBUS;
+	int i;
+	u8 c;
+
+	printf("Bus %d: ", bus);
+
+	/* Select device */
+	mdelay(100);
+	ide_outb(dev, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(dev));
+	mdelay(100);
+	i = 0;
+	do {
+		mdelay(10);
+
+		c = ide_inb(dev, ATA_STATUS);
+		i++;
+		if (i > (ATA_RESET_TIME * 100)) {
+			puts("** Timeout **\n");
+			return -ETIMEDOUT;
+		}
+		if (i >= 100 && !(i % 100))
+			putc('.');
+	} while (c & ATA_STAT_BUSY);
+
+	if (c & (ATA_STAT_BUSY | ATA_STAT_FAULT)) {
+		puts("not available  ");
+		debug("Status = 0x%02X ", c);
+		return -EIO;
+	} else if (IS_ENABLED(CONFIG_ATAPI) && !(c & ATA_STAT_READY)) {
+		/* ATAPI Devices do not set DRDY */
+		puts("not available  ");
+		debug("Status = 0x%02X ", c);
+		return -EIO;
+	}
+	puts("OK ");
+
+	return 0;
+}
+
 static void ide_init(void)
 {
-	unsigned char c;
 	int i, bus;
 
 	schedule();
@@ -694,44 +740,7 @@ static void ide_init(void)
 	 * According to spec, this can take up to 31 seconds!
 	 */
 	for (bus = 0; bus < CONFIG_SYS_IDE_MAXBUS; ++bus) {
-		int dev =
-			bus * (CONFIG_SYS_IDE_MAXDEVICE /
-			       CONFIG_SYS_IDE_MAXBUS);
-
-		printf("Bus %d: ", bus);
-
-		ide_bus_ok[bus] = 0;
-
-		/* Select device */
-		mdelay(100);
-		ide_outb(dev, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(dev));
-		mdelay(100);
-		i = 0;
-		do {
-			mdelay(10);
-
-			c = ide_inb(dev, ATA_STATUS);
-			i++;
-			if (i > (ATA_RESET_TIME * 100)) {
-				puts("** Timeout **\n");
-				return;
-			}
-			if ((i >= 100) && ((i % 100) == 0))
-				putc('.');
-
-		} while (c & ATA_STAT_BUSY);
-
-		if (c & (ATA_STAT_BUSY | ATA_STAT_FAULT)) {
-			puts("not available  ");
-			debug("Status = 0x%02X ", c);
-		} else if (IS_ENABLED(CONFIG_ATAPI) && !(c & ATA_STAT_READY)) {
-			/* ATAPI Devices do not set DRDY */
-			puts("not available  ");
-			debug("Status = 0x%02X ", c);
-		} else {
-			puts("OK ");
-			ide_bus_ok[bus] = 1;
-		}
+		ide_bus_ok[bus] = !ide_init_one(bus);
 		schedule();
 	}
 
-- 
2.40.0.348.gf938b09366-goog

[PATCH 18/30] ide: Make ide_bus_ok[] a local variable

[Simon Glass]

This is only used in one place now, so make it a local variable.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index aac4462f57b9..36c726225d0e 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -39,8 +39,6 @@ ulong ide_bus_offset[CONFIG_SYS_IDE_MAXBUS] = {
 #define ATA_CURR_BASE(dev)	(CONFIG_SYS_ATA_BASE_ADDR + \
 		ide_bus_offset[IDE_BUS(dev)])
 
-static int ide_bus_ok[CONFIG_SYS_IDE_MAXBUS];
-
 struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
 
 #define IDE_TIME_OUT	2000	/* 2 sec timeout */
@@ -52,11 +50,6 @@ struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
 #ifdef CONFIG_IDE_RESET
 static void ide_reset(void)
 {
-	int i;
-
-	for (i = 0; i < CONFIG_SYS_IDE_MAXBUS; ++i)
-		ide_bus_ok[i] = 0;
-
 	ide_set_reset(1);	/* assert reset */
 
 	/* the reset signal shall be asserted for et least 25 us */
@@ -728,6 +721,7 @@ static int ide_init_one(int bus)
 
 static void ide_init(void)
 {
+	bool bus_ok[CONFIG_SYS_IDE_MAXBUS];
 	int i, bus;
 
 	schedule();
@@ -740,7 +734,7 @@ static void ide_init(void)
 	 * According to spec, this can take up to 31 seconds!
 	 */
 	for (bus = 0; bus < CONFIG_SYS_IDE_MAXBUS; ++bus) {
-		ide_bus_ok[bus] = !ide_init_one(bus);
+		bus_ok[bus] = !ide_init_one(bus);
 		schedule();
 	}
 
@@ -755,7 +749,7 @@ static void ide_init(void)
 		ide_dev_desc[i].log2blksz =
 			LOG2_INVALID(typeof(ide_dev_desc[i].log2blksz));
 		ide_dev_desc[i].lba = 0;
-		if (!ide_bus_ok[IDE_BUS(i)])
+		if (!bus_ok[IDE_BUS(i)])
 			continue;
 		ide_ident(&ide_dev_desc[i]);
 		dev_print(&ide_dev_desc[i]);
-- 
2.40.0.348.gf938b09366-goog

[PATCH 19/30] ide: Move setting of vendor strings into ide_probe()

[Simon Glass]

The current implementation adds this information in the block device's
probe() function, which is called in the blk_probe_or_unbind() in
ide_probe().

It is simpler to do this in ide_probe() itself, since the effect is the
same. This helps to consolidate use of ide_dev_desc[] which we would like
to remove.

Use strlcpy() to keep checkpatch happy.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 42 ++++++++++++++++++------------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 36c726225d0e..ecac8b6cfd55 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -987,24 +987,6 @@ ulong ide_or_atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 	return ide_read(dev, blknr, blkcnt, buffer);
 }
 
-static int ide_blk_probe(struct udevice *udev)
-{
-	struct blk_desc *desc = dev_get_uclass_plat(udev);
-
-	/* fill in device vendor/product/rev strings */
-	strncpy(desc->vendor, ide_dev_desc[desc->devnum].vendor,
-		BLK_VEN_SIZE);
-	desc->vendor[BLK_VEN_SIZE] = '\0';
-	strncpy(desc->product, ide_dev_desc[desc->devnum].product,
-		BLK_PRD_SIZE);
-	desc->product[BLK_PRD_SIZE] = '\0';
-	strncpy(desc->revision, ide_dev_desc[desc->devnum].revision,
-		BLK_REV_SIZE);
-	desc->revision[BLK_REV_SIZE] = '\0';
-
-	return 0;
-}
-
 static const struct blk_ops ide_blk_ops = {
 	.read	= ide_or_atapi_read,
 	.write	= ide_write,
@@ -1014,7 +996,6 @@ U_BOOT_DRIVER(ide_blk) = {
 	.name		= "ide_blk",
 	.id		= UCLASS_BLK,
 	.ops		= &ide_blk_ops,
-	.probe		= ide_blk_probe,
 };
 
 static int ide_bootdev_bind(struct udevice *dev)
@@ -1060,17 +1041,19 @@ BOOTDEV_HUNTER(ide_bootdev_hunter) = {
 
 static int ide_probe(struct udevice *udev)
 {
-	struct udevice *blk_dev;
-	char name[20];
-	int blksz;
-	lbaint_t size;
 	int i;
-	int ret;
 
 	ide_init();
 
 	for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
 		if (ide_dev_desc[i].type != DEV_TYPE_UNKNOWN) {
+			struct udevice *blk_dev;
+			struct blk_desc *desc;
+			lbaint_t size;
+			char name[20];
+			int blksz;
+			int ret;
+
 			sprintf(name, "blk#%d", i);
 
 			blksz = ide_dev_desc[i].blksz;
@@ -1095,6 +1078,17 @@ static int ide_probe(struct udevice *udev)
 			ret = bootdev_setup_for_dev(udev, "ide_bootdev");
 			if (ret)
 				return log_msg_ret("bootdev", ret);
+
+			/* fill in device vendor/product/rev strings */
+			desc = dev_get_uclass_plat(blk_dev);
+			strlcpy(desc->vendor, ide_dev_desc[desc->devnum].vendor,
+				BLK_VEN_SIZE);
+			strlcpy(desc->product,
+				ide_dev_desc[desc->devnum].product,
+				BLK_PRD_SIZE);
+			strlcpy(desc->revision,
+				ide_dev_desc[desc->devnum].revision,
+				BLK_REV_SIZE);
 		}
 	}
 
-- 
2.40.0.348.gf938b09366-goog

[PATCH 20/30] ide: Move ide_init() entirely within ide_probe()

[Simon Glass]

Now that ide_probe() is the only caller of ide_init(), move all the code
into the probe function, so it is easier to refactor it.

Move ide_dev_desc[] into ide_probe() to, since it is the only user.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 84 ++++++++++++++++++++-------------------------
 1 file changed, 38 insertions(+), 46 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index ecac8b6cfd55..5fbf144da9d1 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -39,8 +39,6 @@ ulong ide_bus_offset[CONFIG_SYS_IDE_MAXBUS] = {
 #define ATA_CURR_BASE(dev)	(CONFIG_SYS_ATA_BASE_ADDR + \
 		ide_bus_offset[IDE_BUS(dev)])
 
-struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
-
 #define IDE_TIME_OUT	2000	/* 2 sec timeout */
 
 #define ATAPI_TIME_OUT	7000	/* 7 sec timeout (5 sec seems to work...) */
@@ -719,44 +717,6 @@ static int ide_init_one(int bus)
 	return 0;
 }
 
-static void ide_init(void)
-{
-	bool bus_ok[CONFIG_SYS_IDE_MAXBUS];
-	int i, bus;
-
-	schedule();
-
-	/* ATAPI Drives seems to need a proper IDE Reset */
-	ide_reset();
-
-	/*
-	 * Wait for IDE to get ready.
-	 * According to spec, this can take up to 31 seconds!
-	 */
-	for (bus = 0; bus < CONFIG_SYS_IDE_MAXBUS; ++bus) {
-		bus_ok[bus] = !ide_init_one(bus);
-		schedule();
-	}
-
-	putc('\n');
-
-	for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; ++i) {
-		ide_dev_desc[i].type = DEV_TYPE_UNKNOWN;
-		ide_dev_desc[i].uclass_id = UCLASS_IDE;
-		ide_dev_desc[i].devnum = i;
-		ide_dev_desc[i].part_type = PART_TYPE_UNKNOWN;
-		ide_dev_desc[i].blksz = 0;
-		ide_dev_desc[i].log2blksz =
-			LOG2_INVALID(typeof(ide_dev_desc[i].log2blksz));
-		ide_dev_desc[i].lba = 0;
-		if (!bus_ok[IDE_BUS(i)])
-			continue;
-		ide_ident(&ide_dev_desc[i]);
-		dev_print(&ide_dev_desc[i]);
-	}
-	schedule();
-}
-
 static void ide_output_data(int dev, const ulong *sect_buf, int words)
 {
 	uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
@@ -1041,9 +1001,41 @@ BOOTDEV_HUNTER(ide_bootdev_hunter) = {
 
 static int ide_probe(struct udevice *udev)
 {
-	int i;
+	struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
+	bool bus_ok[CONFIG_SYS_IDE_MAXBUS];
+	int i, bus;
+
+	schedule();
+
+	/* ATAPI Drives seems to need a proper IDE Reset */
+	ide_reset();
+
+	/*
+	 * Wait for IDE to get ready.
+	 * According to spec, this can take up to 31 seconds!
+	 */
+	for (bus = 0; bus < CONFIG_SYS_IDE_MAXBUS; ++bus) {
+		bus_ok[bus] = !ide_init_one(bus);
+		schedule();
+	}
 
-	ide_init();
+	putc('\n');
+
+	for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; ++i) {
+		ide_dev_desc[i].type = DEV_TYPE_UNKNOWN;
+		ide_dev_desc[i].uclass_id = UCLASS_IDE;
+		ide_dev_desc[i].devnum = i;
+		ide_dev_desc[i].part_type = PART_TYPE_UNKNOWN;
+		ide_dev_desc[i].blksz = 0;
+		ide_dev_desc[i].log2blksz =
+			LOG2_INVALID(typeof(ide_dev_desc[i].log2blksz));
+		ide_dev_desc[i].lba = 0;
+		if (!bus_ok[IDE_BUS(i)])
+			continue;
+		ide_ident(&ide_dev_desc[i]);
+		dev_print(&ide_dev_desc[i]);
+	}
+	schedule();
 
 	for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
 		if (ide_dev_desc[i].type != DEV_TYPE_UNKNOWN) {
@@ -1075,10 +1067,6 @@ static int ide_probe(struct udevice *udev)
 			if (ret)
 				return ret;
 
-			ret = bootdev_setup_for_dev(udev, "ide_bootdev");
-			if (ret)
-				return log_msg_ret("bootdev", ret);
-
 			/* fill in device vendor/product/rev strings */
 			desc = dev_get_uclass_plat(blk_dev);
 			strlcpy(desc->vendor, ide_dev_desc[desc->devnum].vendor,
@@ -1089,6 +1077,10 @@ static int ide_probe(struct udevice *udev)
 			strlcpy(desc->revision,
 				ide_dev_desc[desc->devnum].revision,
 				BLK_REV_SIZE);
+
+			ret = bootdev_setup_for_dev(udev, "ide_bootdev");
+			if (ret)
+				return log_msg_ret("bootdev", ret);
 		}
 	}
 
-- 
2.40.0.348.gf938b09366-goog

[PATCH 21/30] ide: Combine the two loops in ide_probe()

[Simon Glass]

The two loops in this function operate on the same ide_dev_desc[] array.
Combine them to reduce duplication.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 5fbf144da9d1..d682d6ad9e68 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -1021,7 +1021,12 @@ static int ide_probe(struct udevice *udev)
 
 	putc('\n');
 
-	for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; ++i) {
+	schedule();
+
+	for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
+		if (!bus_ok[IDE_BUS(i)])
+			continue;
+
 		ide_dev_desc[i].type = DEV_TYPE_UNKNOWN;
 		ide_dev_desc[i].uclass_id = UCLASS_IDE;
 		ide_dev_desc[i].devnum = i;
@@ -1030,14 +1035,9 @@ static int ide_probe(struct udevice *udev)
 		ide_dev_desc[i].log2blksz =
 			LOG2_INVALID(typeof(ide_dev_desc[i].log2blksz));
 		ide_dev_desc[i].lba = 0;
-		if (!bus_ok[IDE_BUS(i)])
-			continue;
 		ide_ident(&ide_dev_desc[i]);
 		dev_print(&ide_dev_desc[i]);
-	}
-	schedule();
 
-	for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
 		if (ide_dev_desc[i].type != DEV_TYPE_UNKNOWN) {
 			struct udevice *blk_dev;
 			struct blk_desc *desc;
-- 
2.40.0.348.gf938b09366-goog

[PATCH 22/30] ide: Use desc consistently for struct blk_desc

[Simon Glass]

Most of the code uses 'desc' as the variable name for a blk descriptor.
Change ide to do the same.

Tidy up some extra brackets and types while we are here.

Leave the code in ide_probe() alone since it is about to be refactored.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 101 +++++++++++++++++++++-----------------------
 1 file changed, 47 insertions(+), 54 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index d682d6ad9e68..835e781fccbe 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -414,8 +414,8 @@ error:
 static ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 			void *buffer)
 {
-	struct blk_desc *block_dev = dev_get_uclass_plat(dev);
-	int device = block_dev->devnum;
+	struct blk_desc *desc = dev_get_uclass_plat(dev);
+	int device = desc->devnum;
 	ulong n = 0;
 	unsigned char ccb[12];	/* Command descriptor block */
 	ulong cnt;
@@ -456,15 +456,15 @@ static ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 	return n;
 }
 
-static void atapi_inquiry(struct blk_desc *dev_desc)
+static void atapi_inquiry(struct blk_desc *desc)
 {
 	unsigned char ccb[12];	/* Command descriptor block */
 	unsigned char iobuf[64];	/* temp buf */
 	unsigned char c;
 	int device;
 
-	device = dev_desc->devnum;
-	dev_desc->type = DEV_TYPE_UNKNOWN;	/* not yet valid */
+	device = desc->devnum;
+	desc->type = DEV_TYPE_UNKNOWN;	/* not yet valid */
 
 	memset(ccb, 0, sizeof(ccb));
 	memset(iobuf, 0, sizeof(iobuf));
@@ -478,20 +478,20 @@ static void atapi_inquiry(struct blk_desc *dev_desc)
 		return;
 
 	/* copy device ident strings */
-	ident_cpy((unsigned char *)dev_desc->vendor, &iobuf[8], 8);
-	ident_cpy((unsigned char *)dev_desc->product, &iobuf[16], 16);
-	ident_cpy((unsigned char *)dev_desc->revision, &iobuf[32], 5);
+	ident_cpy((u8 *)desc->vendor, &iobuf[8], 8);
+	ident_cpy((u8 *)desc->product, &iobuf[16], 16);
+	ident_cpy((u8 *)desc->revision, &iobuf[32], 5);
 
-	dev_desc->lun = 0;
-	dev_desc->lba = 0;
-	dev_desc->blksz = 0;
-	dev_desc->log2blksz = LOG2_INVALID(typeof(dev_desc->log2blksz));
-	dev_desc->type = iobuf[0] & 0x1f;
+	desc->lun = 0;
+	desc->lba = 0;
+	desc->blksz = 0;
+	desc->log2blksz = LOG2_INVALID(typeof(desc->log2blksz));
+	desc->type = iobuf[0] & 0x1f;
 
 	if ((iobuf[1] & 0x80) == 0x80)
-		dev_desc->removable = 1;
+		desc->removable = 1;
 	else
-		dev_desc->removable = 0;
+		desc->removable = 0;
 
 	memset(ccb, 0, sizeof(ccb));
 	memset(iobuf, 0, sizeof(iobuf));
@@ -524,19 +524,17 @@ static void atapi_inquiry(struct blk_desc *dev_desc)
 	      iobuf[0], iobuf[1], iobuf[2], iobuf[3],
 	      iobuf[4], iobuf[5], iobuf[6], iobuf[7]);
 
-	dev_desc->lba = ((unsigned long) iobuf[0] << 24) +
-		((unsigned long) iobuf[1] << 16) +
-		((unsigned long) iobuf[2] << 8) + ((unsigned long) iobuf[3]);
-	dev_desc->blksz = ((unsigned long) iobuf[4] << 24) +
-		((unsigned long) iobuf[5] << 16) +
-		((unsigned long) iobuf[6] << 8) + ((unsigned long) iobuf[7]);
-	dev_desc->log2blksz = LOG2(dev_desc->blksz);
+	desc->lba = (ulong)iobuf[0] << 24 | (ulong)iobuf[1] << 16 |
+		(ulong)iobuf[2] << 8 | (ulong)iobuf[3];
+	desc->blksz = (ulong)iobuf[4] << 24 | (ulong)iobuf[5] << 16 |
+		(ulong)iobuf[6] << 8 | (ulong)iobuf[7];
+	desc->log2blksz = LOG2(desc->blksz);
 
 	/* ATAPI devices cannot use 48bit addressing (ATA/ATAPI v7) */
-	dev_desc->lba48 = false;
+	desc->lba48 = false;
 }
 
-static void ide_ident(struct blk_desc *dev_desc)
+static void ide_ident(struct blk_desc *desc)
 {
 	unsigned char c;
 	hd_driveid_t iop;
@@ -544,13 +542,13 @@ static void ide_ident(struct blk_desc *dev_desc)
 	int tries = 1;
 	int device;
 
-	device = dev_desc->devnum;
+	device = desc->devnum;
 	printf("  Device %d: ", device);
 
 	/* Select device
 	 */
 	ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
-	dev_desc->uclass_id = UCLASS_IDE;
+	desc->uclass_id = UCLASS_IDE;
 	if (IS_ENABLED(CONFIG_ATAPI))
 		tries = 2;
 
@@ -610,49 +608,44 @@ static void ide_ident(struct blk_desc *dev_desc)
 
 	ide_input_swap_data(device, (ulong *)&iop, ATA_SECTORWORDS);
 
-	ident_cpy((unsigned char *)dev_desc->revision, iop.fw_rev,
-		  sizeof(dev_desc->revision));
-	ident_cpy((unsigned char *)dev_desc->vendor, iop.model,
-		  sizeof(dev_desc->vendor));
-	ident_cpy((unsigned char *)dev_desc->product, iop.serial_no,
-		  sizeof(dev_desc->product));
+	ident_cpy((u8 *)desc->revision, iop.fw_rev, sizeof(desc->revision));
+	ident_cpy((u8 *)desc->vendor, iop.model, sizeof(desc->vendor));
+	ident_cpy((u8 *)desc->product, iop.serial_no, sizeof(desc->product));
 
 	if ((iop.config & 0x0080) == 0x0080)
-		dev_desc->removable = 1;
+		desc->removable = 1;
 	else
-		dev_desc->removable = 0;
+		desc->removable = 0;
 
 	if (IS_ENABLED(CONFIG_ATAPI) && is_atapi) {
-		dev_desc->atapi = true;
-		atapi_inquiry(dev_desc);
+		desc->atapi = true;
+		atapi_inquiry(desc);
 		return;
 	}
 
 	iop.lba_capacity[0] = be16_to_cpu(iop.lba_capacity[0]);
 	iop.lba_capacity[1] = be16_to_cpu(iop.lba_capacity[1]);
-	dev_desc->lba =
-			((unsigned long)iop.lba_capacity[0]) |
-			((unsigned long)iop.lba_capacity[1] << 16);
+	desc->lba = (ulong)iop.lba_capacity[0] |
+		(ulong)iop.lba_capacity[1] << 16;
 
 	if (IS_ENABLED(CONFIG_LBA48) && (iop.command_set_2 & 0x0400)) {
 		/* LBA 48 support */
-		dev_desc->lba48 = true;
+		desc->lba48 = true;
 		for (int i = 0; i < 4; i++)
 			iop.lba48_capacity[i] = be16_to_cpu(iop.lba48_capacity[i]);
-		dev_desc->lba =
-			((unsigned long long)iop.lba48_capacity[0] |
-			((unsigned long long)iop.lba48_capacity[1] << 16) |
-			((unsigned long long)iop.lba48_capacity[2] << 32) |
-			((unsigned long long)iop.lba48_capacity[3] << 48));
+		desc->lba = (unsigned long long)iop.lba48_capacity[0] |
+			(unsigned long long)iop.lba48_capacity[1] << 16 |
+			(unsigned long long)iop.lba48_capacity[2] << 32 |
+			(unsigned long long)iop.lba48_capacity[3] << 48;
 	} else {
-		dev_desc->lba48 = false;
+		desc->lba48 = false;
 	}
 
 	/* assuming HD */
-	dev_desc->type = DEV_TYPE_HARDDISK;
-	dev_desc->blksz = ATA_BLOCKSIZE;
-	dev_desc->log2blksz = LOG2(dev_desc->blksz);
-	dev_desc->lun = 0;	/* just to fill something in... */
+	desc->type = DEV_TYPE_HARDDISK;
+	desc->blksz = ATA_BLOCKSIZE;
+	desc->log2blksz = LOG2(desc->blksz);
+	desc->lun = 0;	/* just to fill something in... */
 
 #if 0				/* only used to test the powersaving mode,
 				 * if enabled, the drive goes after 5 sec
@@ -751,8 +744,8 @@ static void ide_input_data(int dev, ulong *sect_buf, int words)
 static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 		      void *buffer)
 {
-	struct blk_desc *block_dev = dev_get_uclass_plat(dev);
-	int device = block_dev->devnum;
+	struct blk_desc *desc = dev_get_uclass_plat(dev);
+	int device = desc->devnum;
 	ulong n = 0;
 	unsigned char c;
 	unsigned char pwrsave = 0;	/* power save */
@@ -864,8 +857,8 @@ IDE_READ_E:
 static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 		       const void *buffer)
 {
-	struct blk_desc *block_dev = dev_get_uclass_plat(dev);
-	int device = block_dev->devnum;
+	struct blk_desc *desc = dev_get_uclass_plat(dev);
+	int device = desc->devnum;
 	ulong n = 0;
 	unsigned char c;
 	bool lba48 = false;
-- 
2.40.0.348.gf938b09366-goog

[PATCH 23/30] ide: Make ide_ident() return an error code

[Simon Glass]

Update ide_ident() to indicate whether it finds a device or not. Use
that to decide whether to create a block device for it, rather than
looking DEV_TYPE_UNKNOWN.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 101 +++++++++++++++++++++++---------------------
 1 file changed, 53 insertions(+), 48 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 835e781fccbe..16b119ecbe10 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -534,15 +534,21 @@ static void atapi_inquiry(struct blk_desc *desc)
 	desc->lba48 = false;
 }
 
-static void ide_ident(struct blk_desc *desc)
+/**
+ * ide_ident() - Identify an IDE device
+ *
+ * @device: Device number to use
+ * @desc: Block descriptor to fill in
+ * Returns: 0 if OK, -ENOENT if no device is found
+ */
+static int ide_ident(int device, struct blk_desc *desc)
 {
 	unsigned char c;
 	hd_driveid_t iop;
 	bool is_atapi = false;
 	int tries = 1;
-	int device;
 
-	device = desc->devnum;
+	desc->devnum = device;
 	printf("  Device %d: ", device);
 
 	/* Select device
@@ -604,7 +610,7 @@ static void ide_ident(struct blk_desc *desc)
 	}
 
 	if (!tries)	/* Not found */
-		return;
+		return -ENOENT;
 
 	ide_input_swap_data(device, (ulong *)&iop, ATA_SECTORWORDS);
 
@@ -620,7 +626,7 @@ static void ide_ident(struct blk_desc *desc)
 	if (IS_ENABLED(CONFIG_ATAPI) && is_atapi) {
 		desc->atapi = true;
 		atapi_inquiry(desc);
-		return;
+		return 0;
 	}
 
 	iop.lba_capacity[0] = be16_to_cpu(iop.lba_capacity[0]);
@@ -661,6 +667,8 @@ static void ide_ident(struct blk_desc *desc)
 	udelay(50);
 	c = ide_wait(device, IDE_TIME_OUT);	/* can't take over 500 ms */
 #endif
+
+	return 0;
 }
 
 /**
@@ -1017,64 +1025,61 @@ static int ide_probe(struct udevice *udev)
 	schedule();
 
 	for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
+		struct blk_desc *desc;
+		struct udevice *blk;
+		lbaint_t size;
+		char name[20];
+		int blksz;
+		int ret;
+
 		if (!bus_ok[IDE_BUS(i)])
 			continue;
 
 		ide_dev_desc[i].type = DEV_TYPE_UNKNOWN;
 		ide_dev_desc[i].uclass_id = UCLASS_IDE;
-		ide_dev_desc[i].devnum = i;
 		ide_dev_desc[i].part_type = PART_TYPE_UNKNOWN;
 		ide_dev_desc[i].blksz = 0;
 		ide_dev_desc[i].log2blksz =
 			LOG2_INVALID(typeof(ide_dev_desc[i].log2blksz));
 		ide_dev_desc[i].lba = 0;
-		ide_ident(&ide_dev_desc[i]);
+		ret = ide_ident(i, &ide_dev_desc[i]);
 		dev_print(&ide_dev_desc[i]);
 
-		if (ide_dev_desc[i].type != DEV_TYPE_UNKNOWN) {
-			struct udevice *blk_dev;
-			struct blk_desc *desc;
-			lbaint_t size;
-			char name[20];
-			int blksz;
-			int ret;
+		if (ret)
+			continue;
 
-			sprintf(name, "blk#%d", i);
+		sprintf(name, "blk#%d", i);
 
-			blksz = ide_dev_desc[i].blksz;
-			size = blksz * ide_dev_desc[i].lba;
+		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,
-						 UCLASS_IDE, i,
-						 blksz, size, &blk_dev);
-			if (ret)
-				return ret;
-
-			ret = blk_probe_or_unbind(blk_dev);
-			if (ret)
-				return ret;
-
-			/* fill in device vendor/product/rev strings */
-			desc = dev_get_uclass_plat(blk_dev);
-			strlcpy(desc->vendor, ide_dev_desc[desc->devnum].vendor,
-				BLK_VEN_SIZE);
-			strlcpy(desc->product,
-				ide_dev_desc[desc->devnum].product,
-				BLK_PRD_SIZE);
-			strlcpy(desc->revision,
-				ide_dev_desc[desc->devnum].revision,
-				BLK_REV_SIZE);
-
-			ret = bootdev_setup_for_dev(udev, "ide_bootdev");
-			if (ret)
-				return log_msg_ret("bootdev", ret);
-		}
+		/*
+		 * 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, UCLASS_IDE, i,
+					 blksz, size, &blk);
+		if (ret)
+			return ret;
+
+		ret = blk_probe_or_unbind(blk);
+		if (ret)
+			return ret;
+
+		/* fill in device vendor/product/rev strings */
+		desc = dev_get_uclass_plat(blk);
+		strlcpy(desc->vendor, ide_dev_desc[desc->devnum].vendor,
+			BLK_VEN_SIZE);
+		strlcpy(desc->product, ide_dev_desc[desc->devnum].product,
+			BLK_PRD_SIZE);
+		strlcpy(desc->revision, ide_dev_desc[desc->devnum].revision,
+			BLK_REV_SIZE);
+
+		ret = bootdev_setup_for_dev(udev, "ide_bootdev");
+		if (ret)
+			return log_msg_ret("bootdev", ret);
 	}
 
 	return 0;
-- 
2.40.0.348.gf938b09366-goog

[PATCH 24/30] ide: Move all blk_desc init into ide_ident()

[Simon Glass]

Rather than having the caller fill some of this in, do it all in the
ide_ident() function, since it knows all the values.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 16b119ecbe10..b1c897d6a41b 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -548,13 +548,16 @@ static int ide_ident(int device, struct blk_desc *desc)
 	bool is_atapi = false;
 	int tries = 1;
 
+	memset(desc, '\0', sizeof(*desc));
 	desc->devnum = device;
+	desc->type = DEV_TYPE_UNKNOWN;
+	desc->uclass_id = UCLASS_IDE;
+	desc->log2blksz = LOG2_INVALID(typeof(desc->log2blksz));
 	printf("  Device %d: ", device);
 
 	/* Select device
 	 */
 	ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
-	desc->uclass_id = UCLASS_IDE;
 	if (IS_ENABLED(CONFIG_ATAPI))
 		tries = 2;
 
@@ -1035,13 +1038,6 @@ static int ide_probe(struct udevice *udev)
 		if (!bus_ok[IDE_BUS(i)])
 			continue;
 
-		ide_dev_desc[i].type = DEV_TYPE_UNKNOWN;
-		ide_dev_desc[i].uclass_id = UCLASS_IDE;
-		ide_dev_desc[i].part_type = PART_TYPE_UNKNOWN;
-		ide_dev_desc[i].blksz = 0;
-		ide_dev_desc[i].log2blksz =
-			LOG2_INVALID(typeof(ide_dev_desc[i].log2blksz));
-		ide_dev_desc[i].lba = 0;
 		ret = ide_ident(i, &ide_dev_desc[i]);
 		dev_print(&ide_dev_desc[i]);
 
-- 
2.40.0.348.gf938b09366-goog

[PATCH 25/30] ide: Use a single local blk_desc for ide_ident()

[Simon Glass]

We only use one member of the ide_dev_desc[] array at a time and it does
not stick around outside ide_probe(). Use a single element instead.

Copy over the missing members of blk_desc at the same, since this was
missing from the previous code.

Signed-off-by: Simon Glass <sjg@chromium.org>
Fixes: 68e6f221ed0 ("block: ide: Fix block read/write with driver model")
---

 drivers/block/ide.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index b1c897d6a41b..4c2a6a8e5309 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -1005,7 +1005,6 @@ BOOTDEV_HUNTER(ide_bootdev_hunter) = {
 
 static int ide_probe(struct udevice *udev)
 {
-	struct blk_desc ide_dev_desc[CONFIG_SYS_IDE_MAXDEVICE];
 	bool bus_ok[CONFIG_SYS_IDE_MAXBUS];
 	int i, bus;
 
@@ -1028,7 +1027,7 @@ static int ide_probe(struct udevice *udev)
 	schedule();
 
 	for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
-		struct blk_desc *desc;
+		struct blk_desc *desc, pdesc;
 		struct udevice *blk;
 		lbaint_t size;
 		char name[20];
@@ -1038,16 +1037,16 @@ static int ide_probe(struct udevice *udev)
 		if (!bus_ok[IDE_BUS(i)])
 			continue;
 
-		ret = ide_ident(i, &ide_dev_desc[i]);
-		dev_print(&ide_dev_desc[i]);
+		ret = ide_ident(i, &pdesc);
+		dev_print(&pdesc);
 
 		if (ret)
 			continue;
 
 		sprintf(name, "blk#%d", i);
 
-		blksz = ide_dev_desc[i].blksz;
-		size = blksz * ide_dev_desc[i].lba;
+		blksz = pdesc.blksz;
+		size = blksz * pdesc.lba;
 
 		/*
 		 * With CDROM, if there is no CD inserted, blksz will
@@ -1066,12 +1065,13 @@ static int ide_probe(struct udevice *udev)
 
 		/* fill in device vendor/product/rev strings */
 		desc = dev_get_uclass_plat(blk);
-		strlcpy(desc->vendor, ide_dev_desc[desc->devnum].vendor,
-			BLK_VEN_SIZE);
-		strlcpy(desc->product, ide_dev_desc[desc->devnum].product,
-			BLK_PRD_SIZE);
-		strlcpy(desc->revision, ide_dev_desc[desc->devnum].revision,
-			BLK_REV_SIZE);
+		strlcpy(desc->vendor, pdesc.vendor, BLK_VEN_SIZE);
+		strlcpy(desc->product, pdesc.product, BLK_PRD_SIZE);
+		strlcpy(desc->revision, pdesc.revision, BLK_REV_SIZE);
+		desc->removable = pdesc.removable;
+		desc->atapi = pdesc.atapi;
+		desc->lba48 = pdesc.lba48;
+		desc->type = pdesc.type;
 
 		ret = bootdev_setup_for_dev(udev, "ide_bootdev");
 		if (ret)
-- 
2.40.0.348.gf938b09366-goog

[PATCH 26/30] ide: Correct LBA setting

[Simon Glass]

Fix a longstanding bug where the LBA is calculated as the size of the
media instead of the number of blocks. This was perhaps not noticed
earlier since it prints the correct value first, before setting the wrong
value.

Drop the unnecessary blksz variable while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
Fixes: 68e6f221ed0 ("block: ide: Fix block read/write with driver model")
---

 drivers/block/ide.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 4c2a6a8e5309..72216540d040 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -1029,9 +1029,7 @@ static int ide_probe(struct udevice *udev)
 	for (i = 0; i < CONFIG_SYS_IDE_MAXDEVICE; i++) {
 		struct blk_desc *desc, pdesc;
 		struct udevice *blk;
-		lbaint_t size;
 		char name[20];
-		int blksz;
 		int ret;
 
 		if (!bus_ok[IDE_BUS(i)])
@@ -1045,17 +1043,14 @@ static int ide_probe(struct udevice *udev)
 
 		sprintf(name, "blk#%d", i);
 
-		blksz = pdesc.blksz;
-		size = blksz * pdesc.lba;
-
 		/*
 		 * With CDROM, if there is no CD inserted, blksz will
 		 * be zero, don't bother to create IDE block device.
 		 */
-		if (!blksz)
+		if (!pdesc.blksz)
 			continue;
 		ret = blk_create_devicef(udev, "ide_blk", name, UCLASS_IDE, i,
-					 blksz, size, &blk);
+					 pdesc.blksz, pdesc.lba, &blk);
 		if (ret)
 			return ret;
 
-- 
2.40.0.348.gf938b09366-goog

[PATCH 27/30] ide: Tidy up ide_reset()

[Simon Glass]

Avoid using #ifdef and use a single function declaration, so it is easier
to read.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 72216540d040..fb409338783c 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -45,24 +45,23 @@ ulong ide_bus_offset[CONFIG_SYS_IDE_MAXBUS] = {
 
 #define IDE_SPIN_UP_TIME_OUT 5000 /* 5 sec spin-up timeout */
 
-#ifdef CONFIG_IDE_RESET
 static void ide_reset(void)
 {
-	ide_set_reset(1);	/* assert reset */
+	if (IS_ENABLED(CONFIG_IDE_RESET)) {
+		/* assert reset */
+		ide_set_reset(1);
 
-	/* the reset signal shall be asserted for et least 25 us */
-	udelay(25);
+		/* the reset signal shall be asserted for et least 25 us */
+		udelay(25);
 
-	schedule();
+		schedule();
 
-	/* de-assert RESET signal */
-	ide_set_reset(0);
+		/* de-assert RESET signal */
+		ide_set_reset(0);
 
-	mdelay(250);
+		mdelay(250);
+	}
 }
-#else
-#define ide_reset()	/* dummy */
-#endif /* CONFIG_IDE_RESET */
 
 static void ide_outb(int dev, int port, unsigned char val)
 {
-- 
2.40.0.348.gf938b09366-goog

[PATCH 28/30] ide: Convert to use log_debug()

[Simon Glass]

Avoid the use of the function name in a few of the debug() calls, since
this causes a checkpatch warning. Convert all other calls too.

Use lower-case hex consistently.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 80 ++++++++++++++++++++++-----------------------
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index fb409338783c..37236f6058b4 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -65,8 +65,8 @@ static void ide_reset(void)
 
 static void ide_outb(int dev, int port, unsigned char val)
 {
-	debug("ide_outb (dev= %d, port= 0x%x, val= 0x%02x) : @ 0x%08lx\n",
-	      dev, port, val, ATA_CURR_BASE(dev) + port);
+	log_debug("(dev= %d, port= %#x, val= 0x%02x) : @ 0x%08lx\n",
+		  dev, port, val, ATA_CURR_BASE(dev) + port);
 
 	outb(val, ATA_CURR_BASE(dev) + port);
 }
@@ -77,8 +77,8 @@ static unsigned char ide_inb(int dev, int port)
 
 	val = inb(ATA_CURR_BASE(dev) + port);
 
-	debug("ide_inb (dev= %d, port= 0x%x) : @ 0x%08lx -> 0x%02x\n",
-	      dev, port, ATA_CURR_BASE(dev) + port, val);
+	log_debug("(dev= %d, port= %#x) : @ 0x%08lx -> 0x%02x\n",
+		  dev, port, ATA_CURR_BASE(dev) + port, val);
 	return val;
 }
 
@@ -87,7 +87,7 @@ static void ide_input_swap_data(int dev, ulong *sect_buf, int words)
 	uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
 	ushort *dbuf = (ushort *)sect_buf;
 
-	debug("in input swap data base for read is %p\n", (void *)paddr);
+	log_debug("in input swap data base for read is %p\n", (void *)paddr);
 
 	while (words--) {
 		EIEIO;
@@ -158,7 +158,7 @@ static void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts)
 
 	dbuf = (ushort *)sect_buf;
 
-	debug("in output data shorts base for read is %p\n", (void *)paddr);
+	log_debug("in output data shorts base for read is %p\n", (void *)paddr);
 
 	while (shorts--) {
 		EIEIO;
@@ -173,7 +173,7 @@ static void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts)
 
 	dbuf = (ushort *)sect_buf;
 
-	debug("in input data shorts base for read is %p\n", (void *)paddr);
+	log_debug("in input data shorts base for read is %p\n", (void *)paddr);
 
 	while (shorts--) {
 		EIEIO;
@@ -222,7 +222,7 @@ static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 	ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
 	c = atapi_wait_mask(device, ATAPI_TIME_OUT, mask, res);
 	if ((c & mask) != res) {
-		printf("ATAPI_ISSUE: device %d not ready status %X\n", device,
+		printf("ATAPI_ISSUE: device %d not ready status %x\n", device,
 		       c);
 		err = 0xFF;
 		goto AI_OUT;
@@ -268,8 +268,8 @@ static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 	if ((c & mask) != res) {
 		if (c & ATA_STAT_ERR) {
 			err = (ide_inb(device, ATA_ERROR_REG)) >> 4;
-			debug("atapi_issue 1 returned sense key %X status %02X\n",
-			      err, c);
+			log_debug("1 returned sense key %x status %02x\n",
+				  err, c);
 		} else {
 			printf("ATAPI_ISSUE: (no DRQ) after sending ccb (%x)  status 0x%02x\n",
 			       ccb[0], c);
@@ -292,20 +292,21 @@ static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 		goto AI_OUT;
 	}
 	if (n != buflen) {
-		debug("WARNING, transfer bytes %d not equal with requested %d\n",
-		      n, buflen);
+		log_debug("WARNING, transfer bytes %d not equal with requested %d\n",
+			  n, buflen);
 	}
 	if (n != 0) {		/* data transfer */
-		debug("ATAPI_ISSUE: %d Bytes to transfer\n", n);
+		log_debug("ATAPI_ISSUE: %d Bytes to transfer\n", n);
 		/* we transfer shorts */
 		n >>= 1;
 		/* ok now decide if it is an in or output */
 		if ((ide_inb(device, ATA_SECT_CNT) & 0x02) == 0) {
-			debug("Write to device\n");
+			log_debug("Write to device\n");
 			ide_output_data_shorts(device, (unsigned short *)buffer,
 					       n);
 		} else {
-			debug("Read from device @ %p shorts %d\n", buffer, n);
+			log_debug("Read from device @ %p shorts %d\n", buffer,
+				  n);
 			ide_input_data_shorts(device, (unsigned short *)buffer,
 					      n);
 		}
@@ -316,8 +317,7 @@ static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 	c = atapi_wait_mask(device, ATAPI_TIME_OUT, mask, res);
 	if ((c & ATA_STAT_ERR) == ATA_STAT_ERR) {
 		err = (ide_inb(device, ATA_ERROR_REG) >> 4);
-		debug("atapi_issue 2 returned sense key %X status %X\n", err,
-		      c);
+		log_debug("2 returned sense key %x status %x\n", err, c);
 	} else {
 		err = 0;
 	}
@@ -352,7 +352,7 @@ retry:
 	if (res == 0xFF)
 		return 0xFF;	/* error */
 
-	debug("(auto_req)atapi_issue returned sense key %X\n", res);
+	log_debug("(auto_req)atapi_issue returned sense key %x\n", res);
 
 	memset(sense_ccb, 0, sizeof(sense_ccb));
 	memset(sense_data, 0, sizeof(sense_data));
@@ -364,9 +364,9 @@ retry:
 	asc = (sense_data[12]);
 	ascq = (sense_data[13]);
 
-	debug("ATAPI_CMD_REQ_SENSE returned %x\n", res);
-	debug(" Sense page: %02X key %02X ASC %02X ASCQ %02X\n",
-	      sense_data[0], key, asc, ascq);
+	log_debug("ATAPI_CMD_REQ_SENSE returned %x\n", res);
+	log_debug(" Sense page: %02X key %02X ASC %02X ASCQ %02X\n",
+		  sense_data[0], key, asc, ascq);
 
 	if ((key == 0))
 		return 0;	/* ok device ready */
@@ -390,14 +390,14 @@ retry:
 		goto error;
 	}
 	if (asc == 0x3a) {
-		debug("Media not present\n");
+		log_debug("Media not present\n");
 		goto error;
 	}
 
 	printf("ERROR: Unknown Sense key %02X ASC %02X ASCQ %02X\n", key, asc,
 	       ascq);
 error:
-	debug("ERROR Sense key %02X ASC %02X ASCQ %02X\n", key, asc, ascq);
+	log_debug("ERROR Sense key %02X ASC %02X ASCQ %02X\n", key, asc, ascq);
 	return 0xFF;
 }
 
@@ -419,8 +419,8 @@ static ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 	unsigned char ccb[12];	/* Command descriptor block */
 	ulong cnt;
 
-	debug("atapi_read dev %d start " LBAF " blocks " LBAF
-	      " buffer at %lX\n", device, blknr, blkcnt, (ulong) buffer);
+	log_debug("%d start " LBAF " blocks " LBAF " buffer at %lx\n", device,
+		  blknr, blkcnt, (ulong)buffer);
 
 	do {
 		if (blkcnt > ATAPI_READ_MAX_BLOCK)
@@ -472,7 +472,7 @@ static void atapi_inquiry(struct blk_desc *desc)
 	ccb[4] = 40;		/* allocation Legnth */
 	c = atapi_issue_autoreq(device, ccb, 12, (unsigned char *)iobuf, 40);
 
-	debug("ATAPI_CMD_INQUIRY returned %x\n", c);
+	log_debug("ATAPI_CMD_INQUIRY returned %x\n", c);
 	if (c != 0)
 		return;
 
@@ -499,7 +499,7 @@ static void atapi_inquiry(struct blk_desc *desc)
 
 	c = atapi_issue_autoreq(device, ccb, 12, (unsigned char *)iobuf, 0);
 
-	debug("ATAPI_CMD_START_STOP returned %x\n", c);
+	log_debug("ATAPI_CMD_START_STOP returned %x\n", c);
 	if (c != 0)
 		return;
 
@@ -507,7 +507,7 @@ static void atapi_inquiry(struct blk_desc *desc)
 	memset(iobuf, 0, sizeof(iobuf));
 	c = atapi_issue_autoreq(device, ccb, 12, (unsigned char *)iobuf, 0);
 
-	debug("ATAPI_CMD_UNIT_TEST_READY returned %x\n", c);
+	log_debug("ATAPI_CMD_UNIT_TEST_READY returned %x\n", c);
 	if (c != 0)
 		return;
 
@@ -515,13 +515,13 @@ static void atapi_inquiry(struct blk_desc *desc)
 	memset(iobuf, 0, sizeof(iobuf));
 	ccb[0] = ATAPI_CMD_READ_CAP;
 	c = atapi_issue_autoreq(device, ccb, 12, (unsigned char *)iobuf, 8);
-	debug("ATAPI_CMD_READ_CAP returned %x\n", c);
+	log_debug("ATAPI_CMD_READ_CAP returned %x\n", c);
 	if (c != 0)
 		return;
 
-	debug("Read Cap: LBA %02X%02X%02X%02X blksize %02X%02X%02X%02X\n",
-	      iobuf[0], iobuf[1], iobuf[2], iobuf[3],
-	      iobuf[4], iobuf[5], iobuf[6], iobuf[7]);
+	log_debug("Read Cap: LBA %02X%02X%02X%02X blksize %02X%02X%02X%02X\n",
+		  iobuf[0], iobuf[1], iobuf[2], iobuf[3],
+		  iobuf[4], iobuf[5], iobuf[6], iobuf[7]);
 
 	desc->lba = (ulong)iobuf[0] << 24 | (ulong)iobuf[1] << 16 |
 		(ulong)iobuf[2] << 8 | (ulong)iobuf[3];
@@ -598,7 +598,7 @@ static int ide_ident(int device, struct blk_desc *desc)
 			 * Need to soft reset the device
 			 * in case it's an ATAPI...
 			 */
-			debug("Retrying...\n");
+			log_debug("Retrying...\n");
 			ide_outb(device, ATA_DEV_HD,
 				 ATA_LBA | ATA_DEVICE(device));
 			mdelay(100);
@@ -707,12 +707,12 @@ static int ide_init_one(int bus)
 
 	if (c & (ATA_STAT_BUSY | ATA_STAT_FAULT)) {
 		puts("not available  ");
-		debug("Status = 0x%02X ", c);
+		log_debug("Status = 0x%02X ", c);
 		return -EIO;
 	} else if (IS_ENABLED(CONFIG_ATAPI) && !(c & ATA_STAT_READY)) {
 		/* ATAPI Devices do not set DRDY */
 		puts("not available  ");
-		debug("Status = 0x%02X ", c);
+		log_debug("Status = 0x%02X ", c);
 		return -EIO;
 	}
 	puts("OK ");
@@ -741,7 +741,7 @@ static void ide_input_data(int dev, ulong *sect_buf, int words)
 
 	dbuf = (ushort *)sect_buf;
 
-	debug("in input data base for read is %p\n", (void *)paddr);
+	log_debug("in input data base for read is %p\n", (void *)paddr);
 
 	while (words--) {
 		EIEIO;
@@ -766,8 +766,8 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 		lba48 = true;
 	}
 
-	debug("ide_read dev %d start " LBAF ", blocks " LBAF " buffer at %lX\n",
-	      device, blknr, blkcnt, (ulong) buffer);
+	log_debug("dev %d start " LBAF ", blocks " LBAF " buffer at %lx\n",
+		  device, blknr, blkcnt, (ulong)buffer);
 
 	/* Select device
 	 */
@@ -791,10 +791,10 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 		goto IDE_READ_E;
 	}
 	if ((c & ATA_STAT_ERR) == ATA_STAT_ERR) {
-		printf("No Powersaving mode %X\n", c);
+		printf("No Powersaving mode %x\n", c);
 	} else {
 		c = ide_inb(device, ATA_SECT_CNT);
-		debug("Powersaving %02X\n", c);
+		log_debug("Powersaving %02X\n", c);
 		if (c == 0)
 			pwrsave = 1;
 	}
-- 
2.40.0.348.gf938b09366-goog

[PATCH 29/30] ide: Simplify expressions and hex values

[Simon Glass]

The code has quite a few unnecessary brackets and comparisons to zero,
etc. Fix these up as well as some upper-case hex values and use of 0x in
printf() strings.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 115 +++++++++++++++++++++-----------------------
 1 file changed, 56 insertions(+), 59 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 37236f6058b4..af0c951eb890 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -108,7 +108,7 @@ static uchar ide_wait(int dev, ulong t)
 
 	while ((c = ide_inb(dev, ATA_STATUS)) & ATA_STAT_BUSY) {
 		udelay(100);
-		if (delay-- == 0)
+		if (!delay--)
 			break;
 	}
 	return c;
@@ -153,7 +153,7 @@ OUT:
  * we have our own transfer functions, 2 bytes alligned */
 static void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts)
 {
-	uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
+	uintptr_t paddr = ATA_CURR_BASE(dev) + ATA_DATA_REG;
 	ushort *dbuf;
 
 	dbuf = (ushort *)sect_buf;
@@ -168,7 +168,7 @@ static void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts)
 
 static void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts)
 {
-	uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
+	uintptr_t paddr = ATA_CURR_BASE(dev) + ATA_DATA_REG;
 	ushort *dbuf;
 
 	dbuf = (ushort *)sect_buf;
@@ -195,12 +195,12 @@ static uchar atapi_wait_mask(int dev, ulong t, uchar mask, uchar res)
 	/* prevents to read the status before valid */
 	c = ide_inb(dev, ATA_DEV_CTL);
 
-	while (((c = ide_inb(dev, ATA_STATUS)) & mask) != res) {
+	while (c = ide_inb(dev, ATA_STATUS) & mask, c != res) {
 		/* break if error occurs (doesn't make sense to wait more) */
 		if ((c & ATA_STAT_ERR) == ATA_STAT_ERR)
 			break;
 		udelay(100);
-		if (delay-- == 0)
+		if (!delay--)
 			break;
 	}
 	return c;
@@ -224,16 +224,15 @@ static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 	if ((c & mask) != res) {
 		printf("ATAPI_ISSUE: device %d not ready status %x\n", device,
 		       c);
-		err = 0xFF;
+		err = 0xff;
 		goto AI_OUT;
 	}
 	/* write taskfile */
 	ide_outb(device, ATA_ERROR_REG, 0);	/* no DMA, no overlaped */
 	ide_outb(device, ATA_SECT_CNT, 0);
 	ide_outb(device, ATA_SECT_NUM, 0);
-	ide_outb(device, ATA_CYL_LOW, (unsigned char) (buflen & 0xFF));
-	ide_outb(device, ATA_CYL_HIGH,
-		 (unsigned char) ((buflen >> 8) & 0xFF));
+	ide_outb(device, ATA_CYL_LOW, (unsigned char)(buflen & 0xff));
+	ide_outb(device, ATA_CYL_HIGH, (unsigned char)((buflen >> 8) & 0xff));
 	ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
 
 	ide_outb(device, ATA_COMMAND, ATA_CMD_PACKET);
@@ -244,9 +243,9 @@ static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 	c = atapi_wait_mask(device, ATAPI_TIME_OUT, mask, res);
 
 	if ((c & mask) != res) {	/* DRQ must be 1, BSY 0 */
-		printf("ATAPI_ISSUE: Error (no IRQ) before sending ccb dev %d status 0x%02x\n",
+		printf("ATAPI_ISSUE: Error (no IRQ) before sending ccb dev %d status %#02x\n",
 		       device, c);
-		err = 0xFF;
+		err = 0xff;
 		goto AI_OUT;
 	}
 
@@ -271,9 +270,9 @@ static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 			log_debug("1 returned sense key %x status %02x\n",
 				  err, c);
 		} else {
-			printf("ATAPI_ISSUE: (no DRQ) after sending ccb (%x)  status 0x%02x\n",
+			printf("ATAPI_ISSUE: (no DRQ) after sending ccb (%x)  status %#02x\n",
 			       ccb[0], c);
-			err = 0xFF;
+			err = 0xff;
 		}
 		goto AI_OUT;
 	}
@@ -286,7 +285,7 @@ static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 		err = 0xff;
 		goto AI_OUT;
 	}
-	if ((n == 0) && (buflen < 0)) {
+	if (!n && buflen < 0) {
 		printf("ERROR, transfer bytes %d requested %d\n", n, buflen);
 		err = 0xff;
 		goto AI_OUT;
@@ -295,12 +294,12 @@ static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 		log_debug("WARNING, transfer bytes %d not equal with requested %d\n",
 			  n, buflen);
 	}
-	if (n != 0) {		/* data transfer */
+	if (n) {		/* data transfer */
 		log_debug("ATAPI_ISSUE: %d Bytes to transfer\n", n);
 		/* we transfer shorts */
 		n >>= 1;
 		/* ok now decide if it is an in or output */
-		if ((ide_inb(device, ATA_SECT_CNT) & 0x02) == 0) {
+		if (!(ide_inb(device, ATA_SECT_CNT) & 0x02)) {
 			log_debug("Write to device\n");
 			ide_output_data_shorts(device, (unsigned short *)buffer,
 					       n);
@@ -346,11 +345,11 @@ static unsigned char atapi_issue_autoreq(int device, unsigned char *ccb,
 
 retry:
 	res = atapi_issue(device, ccb, ccblen, buffer, buflen);
-	if (res == 0)
+	if (!res)
 		return 0;	/* Ok */
 
-	if (res == 0xFF)
-		return 0xFF;	/* error */
+	if (res == 0xff)
+		return 0xff;	/* error */
 
 	log_debug("(auto_req)atapi_issue returned sense key %x\n", res);
 
@@ -360,7 +359,7 @@ retry:
 	sense_ccb[4] = 18;	/* allocation Length */
 
 	res = atapi_issue(device, sense_ccb, 12, sense_data, 18);
-	key = (sense_data[2] & 0xF);
+	key = (sense_data[2] & 0xf);
 	asc = (sense_data[12]);
 	ascq = (sense_data[13]);
 
@@ -368,10 +367,10 @@ retry:
 	log_debug(" Sense page: %02X key %02X ASC %02X ASCQ %02X\n",
 		  sense_data[0], key, asc, ascq);
 
-	if ((key == 0))
+	if (!key)
 		return 0;	/* ok device ready */
 
-	if ((key == 6) || (asc == 0x29) || (asc == 0x28)) { /* Unit Attention */
+	if (key == 6 || asc == 0x29 || asc == 0x28) { /* Unit Attention */
 		if (unitattn-- > 0) {
 			mdelay(200);
 			goto retry;
@@ -379,7 +378,7 @@ retry:
 		printf("Unit Attention, tried %d\n", ATAPI_UNIT_ATTN);
 		goto error;
 	}
-	if ((asc == 0x4) && (ascq == 0x1)) {
+	if (asc == 0x4 && ascq == 0x1) {
 		/* not ready, but will be ready soon */
 		if (notready-- > 0) {
 			mdelay(200);
@@ -398,7 +397,7 @@ retry:
 	       ascq);
 error:
 	log_debug("ERROR Sense key %02X ASC %02X ASCQ %02X\n", key, asc, ascq);
-	return 0xFF;
+	return 0xff;
 }
 
 /*
@@ -430,27 +429,25 @@ static ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 
 		ccb[0] = ATAPI_CMD_READ_12;
 		ccb[1] = 0;	/* reserved */
-		ccb[2] = (unsigned char) (blknr >> 24) & 0xFF;	/* MSB Block */
-		ccb[3] = (unsigned char) (blknr >> 16) & 0xFF;	/*  */
-		ccb[4] = (unsigned char) (blknr >> 8) & 0xFF;
-		ccb[5] = (unsigned char) blknr & 0xFF;	/* LSB Block */
-		ccb[6] = (unsigned char) (cnt >> 24) & 0xFF; /* MSB Block cnt */
-		ccb[7] = (unsigned char) (cnt >> 16) & 0xFF;
-		ccb[8] = (unsigned char) (cnt >> 8) & 0xFF;
-		ccb[9] = (unsigned char) cnt & 0xFF;	/* LSB Block */
+		ccb[2] = (unsigned char)(blknr >> 24) & 0xff;	/* MSB Block */
+		ccb[3] = (unsigned char)(blknr >> 16) & 0xff;	/*  */
+		ccb[4] = (unsigned char)(blknr >> 8) & 0xff;
+		ccb[5] = (unsigned char)blknr & 0xff;	/* LSB Block */
+		ccb[6] = (unsigned char)(cnt >> 24) & 0xff; /* MSB Block cnt */
+		ccb[7] = (unsigned char)(cnt >> 16) & 0xff;
+		ccb[8] = (unsigned char)(cnt >> 8) & 0xff;
+		ccb[9] = (unsigned char)cnt & 0xff;	/* LSB Block */
 		ccb[10] = 0;	/* reserved */
 		ccb[11] = 0;	/* reserved */
 
 		if (atapi_issue_autoreq(device, ccb, 12,
 					(unsigned char *)buffer,
-					cnt * ATAPI_READ_BLOCK_SIZE)
-		    == 0xFF) {
+					cnt * ATAPI_READ_BLOCK_SIZE) == 0xff)
 			return n;
-		}
 		n += cnt;
 		blkcnt -= cnt;
 		blknr += cnt;
-		buffer += (cnt * ATAPI_READ_BLOCK_SIZE);
+		buffer += cnt * ATAPI_READ_BLOCK_SIZE;
 	} while (blkcnt > 0);
 	return n;
 }
@@ -473,7 +470,7 @@ static void atapi_inquiry(struct blk_desc *desc)
 	c = atapi_issue_autoreq(device, ccb, 12, (unsigned char *)iobuf, 40);
 
 	log_debug("ATAPI_CMD_INQUIRY returned %x\n", c);
-	if (c != 0)
+	if (c)
 		return;
 
 	/* copy device ident strings */
@@ -487,7 +484,7 @@ static void atapi_inquiry(struct blk_desc *desc)
 	desc->log2blksz = LOG2_INVALID(typeof(desc->log2blksz));
 	desc->type = iobuf[0] & 0x1f;
 
-	if ((iobuf[1] & 0x80) == 0x80)
+	if (iobuf[1] & 0x80)
 		desc->removable = 1;
 	else
 		desc->removable = 0;
@@ -500,7 +497,7 @@ static void atapi_inquiry(struct blk_desc *desc)
 	c = atapi_issue_autoreq(device, ccb, 12, (unsigned char *)iobuf, 0);
 
 	log_debug("ATAPI_CMD_START_STOP returned %x\n", c);
-	if (c != 0)
+	if (c)
 		return;
 
 	memset(ccb, 0, sizeof(ccb));
@@ -508,7 +505,7 @@ static void atapi_inquiry(struct blk_desc *desc)
 	c = atapi_issue_autoreq(device, ccb, 12, (unsigned char *)iobuf, 0);
 
 	log_debug("ATAPI_CMD_UNIT_TEST_READY returned %x\n", c);
-	if (c != 0)
+	if (c)
 		return;
 
 	memset(ccb, 0, sizeof(ccb));
@@ -516,7 +513,7 @@ static void atapi_inquiry(struct blk_desc *desc)
 	ccb[0] = ATAPI_CMD_READ_CAP;
 	c = atapi_issue_autoreq(device, ccb, 12, (unsigned char *)iobuf, 8);
 	log_debug("ATAPI_CMD_READ_CAP returned %x\n", c);
-	if (c != 0)
+	if (c)
 		return;
 
 	log_debug("Read Cap: LBA %02X%02X%02X%02X blksize %02X%02X%02X%02X\n",
@@ -620,7 +617,7 @@ static int ide_ident(int device, struct blk_desc *desc)
 	ident_cpy((u8 *)desc->vendor, iop.model, sizeof(desc->vendor));
 	ident_cpy((u8 *)desc->product, iop.serial_no, sizeof(desc->product));
 
-	if ((iop.config & 0x0080) == 0x0080)
+	if (iop.config & 0x0080)
 		desc->removable = 1;
 	else
 		desc->removable = 0;
@@ -707,12 +704,12 @@ static int ide_init_one(int bus)
 
 	if (c & (ATA_STAT_BUSY | ATA_STAT_FAULT)) {
 		puts("not available  ");
-		log_debug("Status = 0x%02X ", c);
+		log_debug("Status = %#02X ", c);
 		return -EIO;
 	} else if (IS_ENABLED(CONFIG_ATAPI) && !(c & ATA_STAT_READY)) {
 		/* ATAPI Devices do not set DRDY */
 		puts("not available  ");
-		log_debug("Status = 0x%02X ", c);
+		log_debug("Status = %#02X ", c);
 		return -EIO;
 	}
 	puts("OK ");
@@ -795,7 +792,7 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 	} else {
 		c = ide_inb(device, ATA_SECT_CNT);
 		log_debug("Powersaving %02X\n", c);
-		if (c == 0)
+		if (!c)
 			pwrsave = 1;
 	}
 
@@ -810,19 +807,19 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
 			/* write high bits */
 			ide_outb(device, ATA_SECT_CNT, 0);
-			ide_outb(device, ATA_LBA_LOW, (blknr >> 24) & 0xFF);
+			ide_outb(device, ATA_LBA_LOW, (blknr >> 24) & 0xff);
 #ifdef CONFIG_SYS_64BIT_LBA
-			ide_outb(device, ATA_LBA_MID, (blknr >> 32) & 0xFF);
-			ide_outb(device, ATA_LBA_HIGH, (blknr >> 40) & 0xFF);
+			ide_outb(device, ATA_LBA_MID, (blknr >> 32) & 0xff);
+			ide_outb(device, ATA_LBA_HIGH, (blknr >> 40) & 0xff);
 #else
 			ide_outb(device, ATA_LBA_MID, 0);
 			ide_outb(device, ATA_LBA_HIGH, 0);
 #endif
 		}
 		ide_outb(device, ATA_SECT_CNT, 1);
-		ide_outb(device, ATA_LBA_LOW, (blknr >> 0) & 0xFF);
-		ide_outb(device, ATA_LBA_MID, (blknr >> 8) & 0xFF);
-		ide_outb(device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF);
+		ide_outb(device, ATA_LBA_LOW, (blknr >> 0) & 0xff);
+		ide_outb(device, ATA_LBA_MID, (blknr >> 8) & 0xff);
+		ide_outb(device, ATA_LBA_HIGH, (blknr >> 16) & 0xff);
 
 		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
 			ide_outb(device, ATA_DEV_HD,
@@ -831,7 +828,7 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 
 		} else {
 			ide_outb(device, ATA_DEV_HD, ATA_LBA |
-				 ATA_DEVICE(device) | ((blknr >> 24) & 0xF));
+				 ATA_DEVICE(device) | ((blknr >> 24) & 0xf));
 			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ);
 		}
 
@@ -892,19 +889,19 @@ static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
 			/* write high bits */
 			ide_outb(device, ATA_SECT_CNT, 0);
-			ide_outb(device, ATA_LBA_LOW, (blknr >> 24) & 0xFF);
+			ide_outb(device, ATA_LBA_LOW, (blknr >> 24) & 0xff);
 #ifdef CONFIG_SYS_64BIT_LBA
-			ide_outb(device, ATA_LBA_MID, (blknr >> 32) & 0xFF);
-			ide_outb(device, ATA_LBA_HIGH, (blknr >> 40) & 0xFF);
+			ide_outb(device, ATA_LBA_MID, (blknr >> 32) & 0xff);
+			ide_outb(device, ATA_LBA_HIGH, (blknr >> 40) & 0xff);
 #else
 			ide_outb(device, ATA_LBA_MID, 0);
 			ide_outb(device, ATA_LBA_HIGH, 0);
 #endif
 		}
 		ide_outb(device, ATA_SECT_CNT, 1);
-		ide_outb(device, ATA_LBA_LOW, (blknr >> 0) & 0xFF);
-		ide_outb(device, ATA_LBA_MID, (blknr >> 8) & 0xFF);
-		ide_outb(device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF);
+		ide_outb(device, ATA_LBA_LOW, (blknr >> 0) & 0xff);
+		ide_outb(device, ATA_LBA_MID, (blknr >> 8) & 0xff);
+		ide_outb(device, ATA_LBA_HIGH, (blknr >> 16) & 0xff);
 
 		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
 			ide_outb(device, ATA_DEV_HD,
@@ -913,7 +910,7 @@ static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 
 		} else {
 			ide_outb(device, ATA_DEV_HD, ATA_LBA |
-				 ATA_DEVICE(device) | ((blknr >> 24) & 0xF));
+				 ATA_DEVICE(device) | ((blknr >> 24) & 0xf));
 			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_WRITE);
 		}
 
-- 
2.40.0.348.gf938b09366-goog

[PATCH 30/30] ide: Make use of U-Boot types

[Simon Glass]

Use standard U-Boot types in the file to make the code less verbose.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 drivers/block/ide.c | 79 +++++++++++++++++++++------------------------
 1 file changed, 37 insertions(+), 42 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index af0c951eb890..89201dd4d229 100644
--- a/drivers/block/ide.c
+++ b/drivers/block/ide.c
@@ -63,7 +63,7 @@ static void ide_reset(void)
 	}
 }
 
-static void ide_outb(int dev, int port, unsigned char val)
+static void ide_outb(int dev, int port, u8 val)
 {
 	log_debug("(dev= %d, port= %#x, val= 0x%02x) : @ 0x%08lx\n",
 		  dev, port, val, ATA_CURR_BASE(dev) + port);
@@ -71,7 +71,7 @@ static void ide_outb(int dev, int port, unsigned char val)
 	outb(val, ATA_CURR_BASE(dev) + port);
 }
 
-static unsigned char ide_inb(int dev, int port)
+static u8 ide_inb(int dev, int port)
 {
 	uchar val;
 
@@ -119,10 +119,9 @@ static uchar ide_wait(int dev, ulong t)
  * terminate the string
  * "len" is the size of available memory including the terminating '\0'
  */
-static void ident_cpy(unsigned char *dst, unsigned char *src,
-		      unsigned int len)
+static void ident_cpy(u8 *dst, u8 *src, uint len)
 {
-	unsigned char *end, *last;
+	u8 *end, *last;
 
 	last = dst;
 	end = src + len - 1;
@@ -209,10 +208,9 @@ static uchar atapi_wait_mask(int dev, ulong t, uchar mask, uchar res)
 /*
  * issue an atapi command
  */
-static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
-				 unsigned char *buffer, int buflen)
+static u8 atapi_issue(int device, u8 *ccb, int ccblen, u8 *buffer, int buflen)
 {
-	unsigned char c, err, mask, res;
+	u8 c, err, mask, res;
 	int n;
 
 	/* Select device
@@ -231,8 +229,8 @@ static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 	ide_outb(device, ATA_ERROR_REG, 0);	/* no DMA, no overlaped */
 	ide_outb(device, ATA_SECT_CNT, 0);
 	ide_outb(device, ATA_SECT_NUM, 0);
-	ide_outb(device, ATA_CYL_LOW, (unsigned char)(buflen & 0xff));
-	ide_outb(device, ATA_CYL_HIGH, (unsigned char)((buflen >> 8) & 0xff));
+	ide_outb(device, ATA_CYL_LOW, (u8)(buflen & 0xff));
+	ide_outb(device, ATA_CYL_HIGH, (u8)((buflen >> 8) & 0xff));
 	ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
 
 	ide_outb(device, ATA_COMMAND, ATA_CMD_PACKET);
@@ -250,7 +248,7 @@ static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 	}
 
 	/* write command block */
-	ide_output_data_shorts(device, (unsigned short *)ccb, ccblen / 2);
+	ide_output_data_shorts(device, (ushort *)ccb, ccblen / 2);
 
 	/* ATAPI Command written wait for completition */
 	mdelay(5);		/* device must set bsy */
@@ -301,13 +299,11 @@ static unsigned char atapi_issue(int device, unsigned char *ccb, int ccblen,
 		/* ok now decide if it is an in or output */
 		if (!(ide_inb(device, ATA_SECT_CNT) & 0x02)) {
 			log_debug("Write to device\n");
-			ide_output_data_shorts(device, (unsigned short *)buffer,
-					       n);
+			ide_output_data_shorts(device, (ushort *)buffer, n);
 		} else {
 			log_debug("Read from device @ %p shorts %d\n", buffer,
 				  n);
-			ide_input_data_shorts(device, (unsigned short *)buffer,
-					      n);
+			ide_input_data_shorts(device, (ushort *)buffer, n);
 		}
 	}
 	mdelay(5);		/* seems that some CD ROMs need this... */
@@ -332,12 +328,11 @@ AI_OUT:
 #define ATAPI_DRIVE_NOT_READY	100
 #define ATAPI_UNIT_ATTN		10
 
-static unsigned char atapi_issue_autoreq(int device, unsigned char *ccb,
-					 int ccblen,
-					 unsigned char *buffer, int buflen)
+static u8 atapi_issue_autoreq(int device, u8 *ccb, int ccblen, u8 *buffer,
+			      int buflen)
 {
-	unsigned char sense_data[18], sense_ccb[12];
-	unsigned char res, key, asc, ascq;
+	u8 sense_data[18], sense_ccb[12];
+	u8 res, key, asc, ascq;
 	int notready, unitattn;
 
 	unitattn = ATAPI_UNIT_ATTN;
@@ -415,7 +410,7 @@ static ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 	struct blk_desc *desc = dev_get_uclass_plat(dev);
 	int device = desc->devnum;
 	ulong n = 0;
-	unsigned char ccb[12];	/* Command descriptor block */
+	u8 ccb[12];	/* Command descriptor block */
 	ulong cnt;
 
 	log_debug("%d start " LBAF " blocks " LBAF " buffer at %lx\n", device,
@@ -429,19 +424,19 @@ static ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 
 		ccb[0] = ATAPI_CMD_READ_12;
 		ccb[1] = 0;	/* reserved */
-		ccb[2] = (unsigned char)(blknr >> 24) & 0xff;	/* MSB Block */
-		ccb[3] = (unsigned char)(blknr >> 16) & 0xff;	/*  */
-		ccb[4] = (unsigned char)(blknr >> 8) & 0xff;
-		ccb[5] = (unsigned char)blknr & 0xff;	/* LSB Block */
-		ccb[6] = (unsigned char)(cnt >> 24) & 0xff; /* MSB Block cnt */
-		ccb[7] = (unsigned char)(cnt >> 16) & 0xff;
-		ccb[8] = (unsigned char)(cnt >> 8) & 0xff;
-		ccb[9] = (unsigned char)cnt & 0xff;	/* LSB Block */
+		ccb[2] = (u8)(blknr >> 24) & 0xff;	/* MSB Block */
+		ccb[3] = (u8)(blknr >> 16) & 0xff;	/*  */
+		ccb[4] = (u8)(blknr >> 8) & 0xff;
+		ccb[5] = (u8)blknr & 0xff;	/* LSB Block */
+		ccb[6] = (u8)(cnt >> 24) & 0xff; /* MSB Block cnt */
+		ccb[7] = (u8)(cnt >> 16) & 0xff;
+		ccb[8] = (u8)(cnt >> 8) & 0xff;
+		ccb[9] = (u8)cnt & 0xff;	/* LSB Block */
 		ccb[10] = 0;	/* reserved */
 		ccb[11] = 0;	/* reserved */
 
 		if (atapi_issue_autoreq(device, ccb, 12,
-					(unsigned char *)buffer,
+					(u8 *)buffer,
 					cnt * ATAPI_READ_BLOCK_SIZE) == 0xff)
 			return n;
 		n += cnt;
@@ -454,9 +449,9 @@ static ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 
 static void atapi_inquiry(struct blk_desc *desc)
 {
-	unsigned char ccb[12];	/* Command descriptor block */
-	unsigned char iobuf[64];	/* temp buf */
-	unsigned char c;
+	u8 ccb[12];	/* Command descriptor block */
+	u8 iobuf[64];	/* temp buf */
+	u8 c;
 	int device;
 
 	device = desc->devnum;
@@ -467,7 +462,7 @@ static void atapi_inquiry(struct blk_desc *desc)
 
 	ccb[0] = ATAPI_CMD_INQUIRY;
 	ccb[4] = 40;		/* allocation Legnth */
-	c = atapi_issue_autoreq(device, ccb, 12, (unsigned char *)iobuf, 40);
+	c = atapi_issue_autoreq(device, ccb, 12, (u8 *)iobuf, 40);
 
 	log_debug("ATAPI_CMD_INQUIRY returned %x\n", c);
 	if (c)
@@ -494,7 +489,7 @@ static void atapi_inquiry(struct blk_desc *desc)
 	ccb[0] = ATAPI_CMD_START_STOP;
 	ccb[4] = 0x03;		/* start */
 
-	c = atapi_issue_autoreq(device, ccb, 12, (unsigned char *)iobuf, 0);
+	c = atapi_issue_autoreq(device, ccb, 12, (u8 *)iobuf, 0);
 
 	log_debug("ATAPI_CMD_START_STOP returned %x\n", c);
 	if (c)
@@ -502,7 +497,7 @@ static void atapi_inquiry(struct blk_desc *desc)
 
 	memset(ccb, 0, sizeof(ccb));
 	memset(iobuf, 0, sizeof(iobuf));
-	c = atapi_issue_autoreq(device, ccb, 12, (unsigned char *)iobuf, 0);
+	c = atapi_issue_autoreq(device, ccb, 12, (u8 *)iobuf, 0);
 
 	log_debug("ATAPI_CMD_UNIT_TEST_READY returned %x\n", c);
 	if (c)
@@ -511,7 +506,7 @@ static void atapi_inquiry(struct blk_desc *desc)
 	memset(ccb, 0, sizeof(ccb));
 	memset(iobuf, 0, sizeof(iobuf));
 	ccb[0] = ATAPI_CMD_READ_CAP;
-	c = atapi_issue_autoreq(device, ccb, 12, (unsigned char *)iobuf, 8);
+	c = atapi_issue_autoreq(device, ccb, 12, (u8 *)iobuf, 8);
 	log_debug("ATAPI_CMD_READ_CAP returned %x\n", c);
 	if (c)
 		return;
@@ -539,10 +534,10 @@ static void atapi_inquiry(struct blk_desc *desc)
  */
 static int ide_ident(int device, struct blk_desc *desc)
 {
-	unsigned char c;
 	hd_driveid_t iop;
 	bool is_atapi = false;
 	int tries = 1;
+	u8 c;
 
 	memset(desc, '\0', sizeof(*desc));
 	desc->devnum = device;
@@ -753,10 +748,10 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 {
 	struct blk_desc *desc = dev_get_uclass_plat(dev);
 	int device = desc->devnum;
-	ulong n = 0;
-	unsigned char c;
-	unsigned char pwrsave = 0;	/* power save */
 	bool lba48 = false;
+	ulong n = 0;
+	u8 pwrsave = 0;	/* power save */
+	u8 c;
 
 	if (IS_ENABLED(CONFIG_LBA48) && (blknr & 0x0000fffff0000000ULL)) {
 		/* more than 28 bits used, use 48bit mode */
@@ -867,8 +862,8 @@ static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 	struct blk_desc *desc = dev_get_uclass_plat(dev);
 	int device = desc->devnum;
 	ulong n = 0;
-	unsigned char c;
 	bool lba48 = false;
+	u8 c;
 
 	if (IS_ENABLED(CONFIG_LBA48) && (blknr & 0x0000fffff0000000ULL)) {
 		/* more than 28 bits used, use 48bit mode */
-- 
2.40.0.348.gf938b09366-goog

Re: [PATCH 16/30] ide: Avoid preprocessor for CONFIG_LBA48

[Mattijs Korpershoek]

On mar., mars 28, 2023 at 08:07, Simon Glass <sjg@chromium.org> wrote:

> Use IS_ENABLED() instead for all conditions. Add the 'lba48' flag into
> struct blk_desc always, since it uses very little space. Use a bool so
> the meaning is clearer.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  drivers/block/ide.c | 57 ++++++++++++++++-----------------------------
>  include/blk.h       |  4 +---
>  2 files changed, 21 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/block/ide.c b/drivers/block/ide.c
> index 6c5227a5c0e2..45201333c3c5 100644
> --- a/drivers/block/ide.c
> +++ b/drivers/block/ide.c
> @@ -540,11 +540,9 @@ static void atapi_inquiry(struct blk_desc *dev_desc)
>  		((unsigned long) iobuf[5] << 16) +
>  		((unsigned long) iobuf[6] << 8) + ((unsigned long) iobuf[7]);
>  	dev_desc->log2blksz = LOG2(dev_desc->blksz);
> -#ifdef CONFIG_LBA48
> +
>  	/* ATAPI devices cannot use 48bit addressing (ATA/ATAPI v7) */
> -	dev_desc->lba48 = 0;
> -#endif
> -	return;
> +	dev_desc->lba48 = false;
>  }
>  
>  static void ide_ident(struct blk_desc *dev_desc)
> @@ -645,9 +643,9 @@ static void ide_ident(struct blk_desc *dev_desc)
>  			((unsigned long)iop.lba_capacity[0]) |
>  			((unsigned long)iop.lba_capacity[1] << 16);
>  
> -#ifdef CONFIG_LBA48
> -	if (iop.command_set_2 & 0x0400) {	/* LBA 48 support */
> -		dev_desc->lba48 = 1;
> +	if (IS_ENABLED(CONFIG_LBA48) && (iop.command_set_2 & 0x0400)) {
> +		/* LBA 48 support */
> +		dev_desc->lba48 = true;
>  		for (int i = 0; i < 4; i++)
>  			iop.lba48_capacity[i] = be16_to_cpu(iop.lba48_capacity[i]);
>  		dev_desc->lba =
> @@ -656,9 +654,9 @@ static void ide_ident(struct blk_desc *dev_desc)
>  			((unsigned long long)iop.lba48_capacity[2] << 32) |
>  			((unsigned long long)iop.lba48_capacity[3] << 48));
>  	} else {
> -		dev_desc->lba48 = 0;
> +		dev_desc->lba48 = false;
>  	}
> -#endif /* CONFIG_LBA48 */
> +
>  	/* assuming HD */
>  	dev_desc->type = DEV_TYPE_HARDDISK;
>  	dev_desc->blksz = ATA_BLOCKSIZE;
> @@ -795,15 +793,13 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>  	ulong n = 0;
>  	unsigned char c;
>  	unsigned char pwrsave = 0;	/* power save */
> +	bool lba48 = false;
>  
> -#ifdef CONFIG_LBA48
> -	unsigned char lba48 = 0;
> -
> -	if (blknr & 0x0000fffff0000000ULL) {
> +	if (IS_ENABLED(CONFIG_LBA48) && (blknr & 0x0000fffff0000000ULL)) {
>  		/* more than 28 bits used, use 48bit mode */
> -		lba48 = 1;
> +		lba48 = true;
>  	}
> -#endif
> +
>  	debug("ide_read dev %d start " LBAF ", blocks " LBAF " buffer at %lX\n",
>  	      device, blknr, blkcnt, (ulong) buffer);
>  
> @@ -845,8 +841,7 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>  			printf("IDE read: device %d not ready\n", device);
>  			break;
>  		}
> -#ifdef CONFIG_LBA48
> -		if (lba48) {
> +		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
>  			/* write high bits */
>  			ide_outb(device, ATA_SECT_CNT, 0);
>  			ide_outb(device, ATA_LBA_LOW, (blknr >> 24) & 0xFF);
> @@ -858,21 +853,17 @@ static ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>  			ide_outb(device, ATA_LBA_HIGH, 0);
>  #endif
>  		}
> -#endif
>  		ide_outb(device, ATA_SECT_CNT, 1);
>  		ide_outb(device, ATA_LBA_LOW, (blknr >> 0) & 0xFF);
>  		ide_outb(device, ATA_LBA_MID, (blknr >> 8) & 0xFF);
>  		ide_outb(device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF);
>  
> -#ifdef CONFIG_LBA48
> -		if (lba48) {
> +		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
>  			ide_outb(device, ATA_DEV_HD,
>  				 ATA_LBA | ATA_DEVICE(device));
>  			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ_EXT);
>  
> -		} else
> -#endif
> -		{
> +		} else {
>  			ide_outb(device, ATA_DEV_HD, ATA_LBA |
>  				 ATA_DEVICE(device) | ((blknr >> 24) & 0xF));
>  			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ);
> @@ -914,15 +905,12 @@ static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>  	int device = block_dev->devnum;
>  	ulong n = 0;
>  	unsigned char c;
> +	bool lba48 = false;
>  
> -#ifdef CONFIG_LBA48
> -	unsigned char lba48 = 0;
> -
> -	if (blknr & 0x0000fffff0000000ULL) {
> +	if (IS_ENABLED(CONFIG_LBA48) && (blknr & 0x0000fffff0000000ULL)) {
>  		/* more than 28 bits used, use 48bit mode */
> -		lba48 = 1;
> +		lba48 = true;
>  	}
> -#endif
>  
>  	/* Select device
>  	 */
> @@ -935,8 +923,7 @@ static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>  			printf("IDE read: device %d not ready\n", device);
>  			goto WR_OUT;
>  		}
> -#ifdef CONFIG_LBA48
> -		if (lba48) {
> +		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
>  			/* write high bits */
>  			ide_outb(device, ATA_SECT_CNT, 0);
>  			ide_outb(device, ATA_LBA_LOW, (blknr >> 24) & 0xFF);
> @@ -948,21 +935,17 @@ static ulong ide_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>  			ide_outb(device, ATA_LBA_HIGH, 0);
>  #endif
>  		}
> -#endif
>  		ide_outb(device, ATA_SECT_CNT, 1);
>  		ide_outb(device, ATA_LBA_LOW, (blknr >> 0) & 0xFF);
>  		ide_outb(device, ATA_LBA_MID, (blknr >> 8) & 0xFF);
>  		ide_outb(device, ATA_LBA_HIGH, (blknr >> 16) & 0xFF);
>  
> -#ifdef CONFIG_LBA48
> -		if (lba48) {
> +		if (IS_ENABLED(CONFIG_LBA48) && lba48) {
>  			ide_outb(device, ATA_DEV_HD,
>  				 ATA_LBA | ATA_DEVICE(device));
>  			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_WRITE_EXT);
>  
> -		} else
> -#endif
> -		{
> +		} else {
>  			ide_outb(device, ATA_DEV_HD, ATA_LBA |
>  				 ATA_DEVICE(device) | ((blknr >> 24) & 0xF));
>  			ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_WRITE);
> diff --git a/include/blk.h b/include/blk.h
> index 871922dcde07..2c9c7985a885 100644
> --- a/include/blk.h
> +++ b/include/blk.h
> @@ -62,10 +62,8 @@ struct blk_desc {
>  	unsigned char	hwpart;		/* HW partition, e.g. for eMMC */
>  	unsigned char	type;		/* device type */
>  	unsigned char	removable;	/* removable device */
> -#ifdef CONFIG_LBA48
>  	/* device can use 48bit addr (ATA/ATAPI v7) */
> -	unsigned char	lba48;
> -#endif
> +	bool	lba48;

nitpick Is there a reason for having dropped this comment?
/* device can use 48bit addr (ATA/ATAPI v7) */

In any case:

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

>  	unsigned char	atapi;		/* Use ATAPI protocol */
>  	lbaint_t	lba;		/* number of blocks */
>  	unsigned long	blksz;		/* block size */
> -- 
> 2.40.0.348.gf938b09366-goog

Re: [PATCH 10/30] ide: Correct use of ATAPI

[Mattijs Korpershoek]

On mar., mars 28, 2023 at 08:06, Simon Glass <sjg@chromium.org> wrote:

> The use of atapi_read() was incorrect dropped. Fix this so that it will
> be used when needed. Use a udevice for the first argument of atapi_read()
> so it is consistent with ide_read().
>
> This requires much of the ATAPI code to be brought out from behind the
> existing #ifdef. It will still be removed by the compiler if it is not
> needed.
>
> Add an atapi flag to struct blk_desc so the information can be retained.
>
> Fixes: 145df842b44 ("dm: ide: Add support for driver-model block devices")
> Fixes: d0075059e4d ("ide: Drop non-DM code for BLK")
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

> ---
>
>  drivers/block/ide.c | 20 +++++++++++++++++---
>  include/blk.h       |  1 +
>  2 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/ide.c b/drivers/block/ide.c
> index fa5f68ffeb01..875192cba163 100644
> --- a/drivers/block/ide.c
> +++ b/drivers/block/ide.c
> @@ -155,7 +155,6 @@ OUT:
>  	*last = '\0';
>  }
>  
> -#ifdef CONFIG_ATAPI
>  /****************************************************************************
>   * ATAPI Support
>   */
> @@ -422,9 +421,10 @@ error:
>  #define ATAPI_READ_BLOCK_SIZE	2048	/* assuming CD part */
>  #define ATAPI_READ_MAX_BLOCK	(ATAPI_READ_MAX_BYTES/ATAPI_READ_BLOCK_SIZE)
>  
> -ulong atapi_read(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt,
> +ulong atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>  		 void *buffer)
>  {
> +	struct blk_desc *block_dev = dev_get_uclass_plat(dev);
>  	int device = block_dev->devnum;
>  	ulong n = 0;
>  	unsigned char ccb[12];	/* Command descriptor block */
> @@ -466,6 +466,8 @@ ulong atapi_read(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt,
>  	return n;
>  }
>  
> +#ifdef CONFIG_ATAPI
> +
>  static void atapi_inquiry(struct blk_desc *dev_desc)
>  {
>  	unsigned char ccb[12];	/* Command descriptor block */
> @@ -653,6 +655,7 @@ static void ide_ident(struct blk_desc *dev_desc)
>  
>  #ifdef CONFIG_ATAPI
>  	if (is_atapi) {
> +		dev_desc->atapi = true;
>  		atapi_inquiry(dev_desc);
>  		return;
>  	}
> @@ -1010,6 +1013,17 @@ WR_OUT:
>  	return n;
>  }
>  
> +ulong ide_or_atapi_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
> +			void *buffer)
> +{
> +	struct blk_desc *desc = dev_get_uclass_plat(dev);
> +
> +	if (IS_ENABLED(CONFIG_ATAPI) && desc->atapi)
> +		return atapi_read(dev, blknr, blkcnt, buffer);
> +
> +	return ide_read(dev, blknr, blkcnt, buffer);
> +}
> +
>  static int ide_blk_probe(struct udevice *udev)
>  {
>  	struct blk_desc *desc = dev_get_uclass_plat(udev);
> @@ -1029,7 +1043,7 @@ static int ide_blk_probe(struct udevice *udev)
>  }
>  
>  static const struct blk_ops ide_blk_ops = {
> -	.read	= ide_read,
> +	.read	= ide_or_atapi_read,
>  	.write	= ide_write,
>  };
>  
> diff --git a/include/blk.h b/include/blk.h
> index 1db203c1baba..871922dcde07 100644
> --- a/include/blk.h
> +++ b/include/blk.h
> @@ -66,6 +66,7 @@ struct blk_desc {
>  	/* device can use 48bit addr (ATA/ATAPI v7) */
>  	unsigned char	lba48;
>  #endif
> +	unsigned char	atapi;		/* Use ATAPI protocol */
>  	lbaint_t	lba;		/* number of blocks */
>  	unsigned long	blksz;		/* block size */
>  	int		log2blksz;	/* for convenience: log2(blksz) */
> -- 
> 2.40.0.348.gf938b09366-goog

Re: [PATCH 16/30] ide: Avoid preprocessor for CONFIG_LBA48

[Simon Glass]

Hi Mattijs,

On Tue, 28 Mar 2023 at 08:18, Mattijs Korpershoek
<mkorpershoek@baylibre.com> wrote:
>
> On mar., mars 28, 2023 at 08:07, Simon Glass <sjg@chromium.org> wrote:
>
> > Use IS_ENABLED() instead for all conditions. Add the 'lba48' flag into
> > struct blk_desc always, since it uses very little space. Use a bool so
> > the meaning is clearer.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  drivers/block/ide.c | 57 ++++++++++++++++-----------------------------
> >  include/blk.h       |  4 +---
> >  2 files changed, 21 insertions(+), 40 deletions(-)
> >
[..]

> > diff --git a/include/blk.h b/include/blk.h
> > index 871922dcde07..2c9c7985a885 100644
> > --- a/include/blk.h
> > +++ b/include/blk.h
> > @@ -62,10 +62,8 @@ struct blk_desc {
> >       unsigned char   hwpart;         /* HW partition, e.g. for eMMC */
> >       unsigned char   type;           /* device type */
> >       unsigned char   removable;      /* removable device */
> > -#ifdef CONFIG_LBA48
> >       /* device can use 48bit addr (ATA/ATAPI v7) */
> > -     unsigned char   lba48;
> > -#endif
> > +     bool    lba48;
>
> nitpick Is there a reason for having dropped this comment?
> /* device can use 48bit addr (ATA/ATAPI v7) */
>
> In any case:
>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Thanks for the review. On closer inspection it does not actually drop
the comment.

Regards,
Simon

Re: [PATCH 00/30] ide: Clean up code and fix a few bugs

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 590 bytes --]

On Tue, Mar 28, 2023 at 08:06:47AM +1300, Simon Glass wrote:

> This code was converted to driver model a long time again but it was a
> pretty rough conversion. It introduced a few minor bugs, e.g. the device
> capacity is incorrect and some flags are lost (such as lba48).
> 
> This series tidies up the code and fixes these bugs. This involves quite
> a bit of refactoring, so it is done one patch at a time for easier
> review.

This doesn't apply cleanly currently, and my first attempt at applying
resulted in fails to build, please rebase and resend, thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]