[PATCH] Fix IDE commands issued, fix endian issues, fix non MMIO
Fixes IDE issues found on the Malta board under Qemu:
1) DMA implied commands were sent to the controller in stead of the PIO variants. The rest of the code is DMA free and written for PIO operation.
2) direct pointer access was used to read and write the registers instead of the inb/inw/outb/outw functions/macros. Registers don't have to be memory mapped and ATA_CURR_BASE() does not have to return an offset from address zero.
3) Endian isues in ide_ident() and reading/writing data in general. Names were corrupted and sizes misreported.
Tested malta_defconfig and maltael_defconfig to work again in Qemu.
Signed-off-by: Reinoud Zandijk reinoud@NetBSD.org
--- drivers/block/ide.c | 152 +++++++++++++------------------------------- include/ata.h | 2 +- 2 files changed, 44 insertions(+), 110 deletions(-)
diff --git a/drivers/block/ide.c b/drivers/block/ide.c index 43a0776099..862a85bc87 100644 --- a/drivers/block/ide.c +++ b/drivers/block/ide.c @@ -130,56 +130,38 @@ OUT: * ATAPI Support */
-#if defined(CONFIG_IDE_SWAP_IO) /* 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) { + uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); ushort *dbuf; - volatile ushort *pbuf;
- pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); dbuf = (ushort *)sect_buf;
- debug("in output data shorts base for read is %lx\n", - (unsigned long) pbuf); + debug("in output data shorts base for read is %p\n", (void *)paddr);
while (shorts--) { EIEIO; - *pbuf = *dbuf++; + outw(cpu_to_le16(*dbuf++), paddr); } }
__weak void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts) { + uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); ushort *dbuf; - volatile ushort *pbuf;
- pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); dbuf = (ushort *)sect_buf;
- debug("in input data shorts base for read is %lx\n", - (unsigned long) pbuf); + debug("in input data shorts base for read is %p\n", (void *)paddr);
while (shorts--) { EIEIO; - *dbuf++ = *pbuf; + *dbuf++ = le16_to_cpu(inw(paddr)); } }
-#else /* ! CONFIG_IDE_SWAP_IO */ -__weak void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts) -{ - outsw(ATA_CURR_BASE(dev) + ATA_DATA_REG, sect_buf, shorts); -} - -__weak void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts) -{ - insw(ATA_CURR_BASE(dev) + ATA_DATA_REG, sect_buf, shorts); -} - -#endif /* CONFIG_IDE_SWAP_IO */ - /* * Wait until (Status & mask) == res, or timeout (in ms) * Return last status @@ -636,19 +618,6 @@ static void ide_ident(struct blk_desc *dev_desc) sizeof(dev_desc->vendor)); ident_cpy((unsigned char *)dev_desc->product, iop.serial_no, sizeof(dev_desc->product)); -#ifdef __LITTLE_ENDIAN - /* - * firmware revision, model, and serial number have Big Endian Byte - * order in Word. Convert all three to little endian. - * - * See CF+ and CompactFlash Specification Revision 2.0: - * 6.2.1.6: Identify Drive, Table 39 for more details - */ - - strswab(dev_desc->revision); - strswab(dev_desc->vendor); - strswab(dev_desc->product); -#endif /* __LITTLE_ENDIAN */
if ((iop.config & 0x0080) == 0x0080) dev_desc->removable = 1; @@ -662,26 +631,22 @@ static void ide_ident(struct blk_desc *dev_desc) } #endif /* CONFIG_ATAPI */
-#ifdef __BIG_ENDIAN - /* swap shorts */ - dev_desc->lba = (iop.lba_capacity << 16) | (iop.lba_capacity >> 16); -#else /* ! __BIG_ENDIAN */ - /* - * do not swap shorts on little endian - * - * See CF+ and CompactFlash Specification Revision 2.0: - * 6.2.1.6: Identfy Drive, Table 39, Word Address 57-58 for details. - */ - dev_desc->lba = iop.lba_capacity; -#endif /* __BIG_ENDIAN */ + 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);
#ifdef CONFIG_LBA48 if (iop.command_set_2 & 0x0400) { /* LBA 48 support */ dev_desc->lba48 = 1; - 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); + 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)); } else { dev_desc->lba48 = 0; } @@ -846,90 +811,59 @@ void ide_init(void) #endif }
-/* We only need to swap data if we are running on a big endian cpu. */ -#if defined(__LITTLE_ENDIAN) __weak void ide_input_swap_data(int dev, ulong *sect_buf, int words) { - ide_input_data(dev, sect_buf, words); -} -#else -__weak void ide_input_swap_data(int dev, ulong *sect_buf, int words) -{ - volatile ushort *pbuf = - (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); + uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); ushort *dbuf = (ushort *)sect_buf;
- debug("in input swap data base for read is %lx\n", - (unsigned long) pbuf); + debug("in input swap data base for read is %p\n", (void *)paddr);
while (words--) { -#ifdef __MIPS__ - *dbuf++ = swab16p((u16 *)pbuf); - *dbuf++ = swab16p((u16 *)pbuf); -#else - *dbuf++ = ld_le16(pbuf); - *dbuf++ = ld_le16(pbuf); -#endif /* !MIPS */ + EIEIO; + *dbuf++ = be16_to_cpu(inw(paddr)); + EIEIO; + *dbuf++ = be16_to_cpu(inw(paddr)); } } -#endif /* __LITTLE_ENDIAN */ -
-#if defined(CONFIG_IDE_SWAP_IO) __weak void ide_output_data(int dev, const ulong *sect_buf, int words) { +#if defined(CONFIG_IDE_AHB) + ide_write_data(dev, sect_buf, words); +#else + uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); ushort *dbuf; - volatile ushort *pbuf;
- pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); dbuf = (ushort *)sect_buf; while (words--) { EIEIO; - *pbuf = *dbuf++; + outw(cpu_to_le16(*dbuf++), paddr); EIEIO; - *pbuf = *dbuf++; + outw(cpu_to_le16(*dbuf++), paddr); } +#endif /* CONFIG_IDE_AHB */ } -#else /* ! CONFIG_IDE_SWAP_IO */ -__weak void ide_output_data(int dev, const ulong *sect_buf, int words) -{ -#if defined(CONFIG_IDE_AHB) - ide_write_data(dev, sect_buf, words); -#else - outsw(ATA_CURR_BASE(dev) + ATA_DATA_REG, sect_buf, words << 1); -#endif -} -#endif /* CONFIG_IDE_SWAP_IO */
-#if defined(CONFIG_IDE_SWAP_IO) __weak void ide_input_data(int dev, ulong *sect_buf, int words) { +#if defined(CONFIG_IDE_AHB) + ide_read_data(dev, sect_buf, words); +#else + uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); ushort *dbuf; - volatile ushort *pbuf;
- pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); dbuf = (ushort *)sect_buf;
- debug("in input data base for read is %lx\n", (unsigned long) pbuf); + debug("in input data base for read is %p\n", (void *)paddr);
while (words--) { EIEIO; - *dbuf++ = *pbuf; + *dbuf++ = le16_to_cpu(inw(paddr)); EIEIO; - *dbuf++ = *pbuf; + *dbuf++ = le16_to_cpu(inw(paddr)); } +#endif /* CONFIG_IDE_AHB */ } -#else /* ! CONFIG_IDE_SWAP_IO */ -__weak void ide_input_data(int dev, ulong *sect_buf, int words) -{ -#if defined(CONFIG_IDE_AHB) - ide_read_data(dev, sect_buf, words); -#else - insw(ATA_CURR_BASE(dev) + ATA_DATA_REG, sect_buf, words << 1); -#endif -} - -#endif /* CONFIG_IDE_SWAP_IO */
#ifdef CONFIG_BLK ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt, @@ -1019,14 +953,14 @@ ulong ide_read(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt, if (lba48) { ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device)); - ide_outb(device, ATA_COMMAND, ATA_CMD_READ_EXT); + ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ_EXT);
} else #endif { ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device) | ((blknr >> 24) & 0xF)); - ide_outb(device, ATA_COMMAND, ATA_CMD_READ); + ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ); }
udelay(50); @@ -1116,14 +1050,14 @@ ulong ide_write(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt, if (lba48) { ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device)); - ide_outb(device, ATA_COMMAND, ATA_CMD_WRITE_EXT); + ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_WRITE_EXT);
} else #endif { ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device) | ((blknr >> 24) & 0xF)); - ide_outb(device, ATA_COMMAND, ATA_CMD_WRITE); + ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_WRITE); }
udelay(50); diff --git a/include/ata.h b/include/ata.h index 3d870c973f..32ad5f6427 100644 --- a/include/ata.h +++ b/include/ata.h @@ -134,7 +134,7 @@ typedef struct hd_driveid { unsigned short cur_capacity1; /* (2 words, misaligned int) */ unsigned char multsect; /* current multiple sector count */ unsigned char multsect_valid; /* when (bit0==1) multsect is ok */ - unsigned int lba_capacity; /* total number of sectors */ + unsigned short lba_capacity[2];/* two words containing total number of sectors */ unsigned short dma_1word; /* single-word dma info */ unsigned short dma_mword; /* multiple-word dma info */ unsigned short eide_pio_modes; /* bits 0:mode3 1:mode4 */
On 24.02.21 17:44, Reinoud Zandijk wrote:
Fixes IDE issues found on the Malta board under Qemu:
- DMA implied commands were sent to the controller in stead of the PIO
variants. The rest of the code is DMA free and written for PIO operation.
- direct pointer access was used to read and write the registers instead
of the inb/inw/outb/outw functions/macros. Registers don't have to be memory mapped and ATA_CURR_BASE() does not have to return an offset from address zero.
- Endian isues in ide_ident() and reading/writing data in general. Names
were corrupted and sizes misreported.
It is preferable to have each issue fixed in a separate patch.
Tested malta_defconfig and maltael_defconfig to work again in Qemu.
What about the other architectures which can use the driver?
@Simon: Can we get rid of U_BOOT_LEGACY_BLK(ide)?
Best regards
Heinrich
Signed-off-by: Reinoud Zandijk reinoud@NetBSD.org
drivers/block/ide.c | 152 +++++++++++++------------------------------- include/ata.h | 2 +- 2 files changed, 44 insertions(+), 110 deletions(-)
diff --git a/drivers/block/ide.c b/drivers/block/ide.c index 43a0776099..862a85bc87 100644 --- a/drivers/block/ide.c +++ b/drivers/block/ide.c @@ -130,56 +130,38 @@ OUT:
- ATAPI Support
*/
-#if defined(CONFIG_IDE_SWAP_IO) /* 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) {
- uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); ushort *dbuf;
volatile ushort *pbuf;
pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); dbuf = (ushort *)sect_buf;
debug("in output data shorts base for read is %lx\n",
(unsigned long) pbuf);
debug("in output data shorts base for read is %p\n", (void *)paddr);
while (shorts--) { EIEIO;
*pbuf = *dbuf++;
outw(cpu_to_le16(*dbuf++), paddr);}
__weak void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts) {
- uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); ushort *dbuf;
volatile ushort *pbuf;
pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); dbuf = (ushort *)sect_buf;
debug("in input data shorts base for read is %lx\n",
(unsigned long) pbuf);
debug("in input data shorts base for read is %p\n", (void *)paddr);
while (shorts--) { EIEIO;
*dbuf++ = *pbuf;
*dbuf++ = le16_to_cpu(inw(paddr));}
-#else /* ! CONFIG_IDE_SWAP_IO */ -__weak void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts) -{
- outsw(ATA_CURR_BASE(dev) + ATA_DATA_REG, sect_buf, shorts);
-}
-__weak void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts) -{
- insw(ATA_CURR_BASE(dev) + ATA_DATA_REG, sect_buf, shorts);
-}
-#endif /* CONFIG_IDE_SWAP_IO */
/*
- Wait until (Status & mask) == res, or timeout (in ms)
- Return last status
@@ -636,19 +618,6 @@ static void ide_ident(struct blk_desc *dev_desc) sizeof(dev_desc->vendor)); ident_cpy((unsigned char *)dev_desc->product, iop.serial_no, sizeof(dev_desc->product)); -#ifdef __LITTLE_ENDIAN
- /*
* firmware revision, model, and serial number have Big Endian Byte* order in Word. Convert all three to little endian.** See CF+ and CompactFlash Specification Revision 2.0:* 6.2.1.6: Identify Drive, Table 39 for more details*/- strswab(dev_desc->revision);
- strswab(dev_desc->vendor);
- strswab(dev_desc->product);
-#endif /* __LITTLE_ENDIAN */
if ((iop.config & 0x0080) == 0x0080) dev_desc->removable = 1; @@ -662,26 +631,22 @@ static void ide_ident(struct blk_desc *dev_desc) } #endif /* CONFIG_ATAPI */
-#ifdef __BIG_ENDIAN
- /* swap shorts */
- dev_desc->lba = (iop.lba_capacity << 16) | (iop.lba_capacity >> 16);
-#else /* ! __BIG_ENDIAN */
- /*
* do not swap shorts on little endian** See CF+ and CompactFlash Specification Revision 2.0:* 6.2.1.6: Identfy Drive, Table 39, Word Address 57-58 for details.*/- dev_desc->lba = iop.lba_capacity;
-#endif /* __BIG_ENDIAN */
- 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);#ifdef CONFIG_LBA48 if (iop.command_set_2 & 0x0400) { /* LBA 48 support */ dev_desc->lba48 = 1;
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);
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));@@ -846,90 +811,59 @@ void ide_init(void) #endif }
-/* We only need to swap data if we are running on a big endian cpu. */ -#if defined(__LITTLE_ENDIAN) __weak void ide_input_swap_data(int dev, ulong *sect_buf, int words) {
- ide_input_data(dev, sect_buf, words);
-} -#else -__weak void ide_input_swap_data(int dev, ulong *sect_buf, int words) -{
- volatile ushort *pbuf =
(ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG);
- uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); ushort *dbuf = (ushort *)sect_buf;
- debug("in input swap data base for read is %lx\n",
(unsigned long) pbuf);
debug("in input swap data base for read is %p\n", (void *)paddr);
while (words--) {
-#ifdef __MIPS__
*dbuf++ = swab16p((u16 *)pbuf);*dbuf++ = swab16p((u16 *)pbuf);-#else
*dbuf++ = ld_le16(pbuf);*dbuf++ = ld_le16(pbuf);-#endif /* !MIPS */
EIEIO;*dbuf++ = be16_to_cpu(inw(paddr));EIEIO;*dbuf++ = be16_to_cpu(inw(paddr));} -#endif /* __LITTLE_ENDIAN */
-#if defined(CONFIG_IDE_SWAP_IO) __weak void ide_output_data(int dev, const ulong *sect_buf, int words) { +#if defined(CONFIG_IDE_AHB)
- ide_write_data(dev, sect_buf, words);
+#else
- uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); ushort *dbuf;
volatile ushort *pbuf;
pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); dbuf = (ushort *)sect_buf; while (words--) { EIEIO;
*pbuf = *dbuf++;
outw(cpu_to_le16(*dbuf++), paddr);
*pbuf = *dbuf++;
outw(cpu_to_le16(*dbuf++), paddr);+#endif /* CONFIG_IDE_AHB */ } -#else /* ! CONFIG_IDE_SWAP_IO */ -__weak void ide_output_data(int dev, const ulong *sect_buf, int words) -{ -#if defined(CONFIG_IDE_AHB)
- ide_write_data(dev, sect_buf, words);
-#else
- outsw(ATA_CURR_BASE(dev) + ATA_DATA_REG, sect_buf, words << 1);
-#endif -} -#endif /* CONFIG_IDE_SWAP_IO */
-#if defined(CONFIG_IDE_SWAP_IO) __weak void ide_input_data(int dev, ulong *sect_buf, int words) { +#if defined(CONFIG_IDE_AHB)
- ide_read_data(dev, sect_buf, words);
+#else
- uintptr_t paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); ushort *dbuf;
volatile ushort *pbuf;
pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); dbuf = (ushort *)sect_buf;
debug("in input data base for read is %lx\n", (unsigned long) pbuf);
debug("in input data base for read is %p\n", (void *)paddr);
while (words--) { EIEIO;
*dbuf++ = *pbuf;
*dbuf++ = le16_to_cpu(inw(paddr));
*dbuf++ = *pbuf;
*dbuf++ = le16_to_cpu(inw(paddr));+#endif /* CONFIG_IDE_AHB */ } -#else /* ! CONFIG_IDE_SWAP_IO */ -__weak void ide_input_data(int dev, ulong *sect_buf, int words) -{ -#if defined(CONFIG_IDE_AHB)
- ide_read_data(dev, sect_buf, words);
-#else
- insw(ATA_CURR_BASE(dev) + ATA_DATA_REG, sect_buf, words << 1);
-#endif -}
-#endif /* CONFIG_IDE_SWAP_IO */
#ifdef CONFIG_BLK ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt, @@ -1019,14 +953,14 @@ ulong ide_read(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt, if (lba48) { ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
ide_outb(device, ATA_COMMAND, ATA_CMD_READ_EXT);
ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ_EXT);} else
#endif { ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device) | ((blknr >> 24) & 0xF));
ide_outb(device, ATA_COMMAND, ATA_CMD_READ);
ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ);}
udelay(50);
@@ -1116,14 +1050,14 @@ ulong ide_write(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt, if (lba48) { ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device));
ide_outb(device, ATA_COMMAND, ATA_CMD_WRITE_EXT);
ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_WRITE_EXT);} else
#endif { ide_outb(device, ATA_DEV_HD, ATA_LBA | ATA_DEVICE(device) | ((blknr >> 24) & 0xF));
ide_outb(device, ATA_COMMAND, ATA_CMD_WRITE);
ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_WRITE);}
udelay(50);
diff --git a/include/ata.h b/include/ata.h index 3d870c973f..32ad5f6427 100644 --- a/include/ata.h +++ b/include/ata.h @@ -134,7 +134,7 @@ typedef struct hd_driveid { unsigned short cur_capacity1; /* (2 words, misaligned int) */ unsigned char multsect; /* current multiple sector count */ unsigned char multsect_valid; /* when (bit0==1) multsect is ok */
- unsigned int lba_capacity; /* total number of sectors */
- unsigned short lba_capacity[2];/* two words containing total number of sectors */ unsigned short dma_1word; /* single-word dma info */ unsigned short dma_mword; /* multiple-word dma info */ unsigned short eide_pio_modes; /* bits 0:mode3 1:mode4 */
On Wed, Feb 24, 2021 at 06:57:08PM +0100, Heinrich Schuchardt wrote:
On 24.02.21 17:44, Reinoud Zandijk wrote:
Fixes IDE issues found on the Malta board under Qemu:
- DMA implied commands were sent to the controller in stead of the PIO
variants. The rest of the code is DMA free and written for PIO operation.
- direct pointer access was used to read and write the registers instead
of the inb/inw/outb/outw functions/macros. Registers don't have to be memory mapped and ATA_CURR_BASE() does not have to return an offset from address zero.
- Endian isues in ide_ident() and reading/writing data in general. Names
were corrupted and sizes misreported.
It is preferable to have each issue fixed in a separate patch.
Tested malta_defconfig and maltael_defconfig to work again in Qemu.
What about the other architectures which can use the driver?
@Simon: Can we get rid of U_BOOT_LEGACY_BLK(ide)?
The various "remove ..." board series I've been sending cover the block case too, so after v2021.04 and once I've applied the "remove ..." series stuff, yes, I think so.
Dear Heinrich,
On Wed, Feb 24, 2021 at 06:57:08PM +0100, Heinrich Schuchardt wrote:
On 24.02.21 17:44, Reinoud Zandijk wrote:
Fixes IDE issues found on the Malta board under Qemu:
- DMA implied commands were sent to the controller in stead of the PIO
variants. The rest of the code is DMA free and written for PIO operation.
- direct pointer access was used to read and write the registers instead
of the inb/inw/outb/outw functions/macros. Registers don't have to be memory mapped and ATA_CURR_BASE() does not have to return an offset from address zero.
- Endian isues in ide_ident() and reading/writing data in general. Names
were corrupted and sizes misreported.
It is preferable to have each issue fixed in a separate patch.
They are related; 2 and 3 are really one and 1 could in theory be separate but it won't work on its own.
Tested malta_defconfig and maltael_defconfig to work again in Qemu.
What about the other architectures which can use the driver?
As for testing, I compile tested an ARM board, edminiv2, successfully, but don't know how to invoke qemu for it. As for running the tests, my setup is not capable of running the tests since they seem to be for linux only.
Does the automatic testing check if disks are indeed found correctly and checking reading or writing to a disk image?
With regards, Reinoud
ping?
On Wed, Feb 24, 2021 at 10:55:07PM +0100, Reinoud Zandijk wrote:
Dear Heinrich,
On Wed, Feb 24, 2021 at 06:57:08PM +0100, Heinrich Schuchardt wrote:
On 24.02.21 17:44, Reinoud Zandijk wrote:
Fixes IDE issues found on the Malta board under Qemu:
- DMA implied commands were sent to the controller in stead of the PIO
variants. The rest of the code is DMA free and written for PIO operation.
- direct pointer access was used to read and write the registers instead
of the inb/inw/outb/outw functions/macros. Registers don't have to be memory mapped and ATA_CURR_BASE() does not have to return an offset from address zero.
- Endian isues in ide_ident() and reading/writing data in general. Names
were corrupted and sizes misreported.
It is preferable to have each issue fixed in a separate patch.
They are related; 2 and 3 are really one and 1 could in theory be separate but it won't work on its own.
Tested malta_defconfig and maltael_defconfig to work again in Qemu.
What about the other architectures which can use the driver?
As for testing, I compile tested an ARM board, edminiv2, successfully, but don't know how to invoke qemu for it. As for running the tests, my setup is not capable of running the tests since they seem to be for linux only.
Does the automatic testing check if disks are indeed found correctly and checking reading or writing to a disk image?
With regards, Reinoud
ping?
On Wed, Feb 24, 2021 at 10:55:07PM +0100, Reinoud Zandijk wrote:
Dear Heinrich,
On Wed, Feb 24, 2021 at 06:57:08PM +0100, Heinrich Schuchardt wrote:
On 24.02.21 17:44, Reinoud Zandijk wrote:
Fixes IDE issues found on the Malta board under Qemu:
- DMA implied commands were sent to the controller in stead of the PIO
variants. The rest of the code is DMA free and written for PIO operation.
- direct pointer access was used to read and write the registers instead
of the inb/inw/outb/outw functions/macros. Registers don't have to be memory mapped and ATA_CURR_BASE() does not have to return an offset from address zero.
- Endian isues in ide_ident() and reading/writing data in general. Names
were corrupted and sizes misreported.
It is preferable to have each issue fixed in a separate patch.
They are related; 2 and 3 are really one and 1 could in theory be separate but it won't work on its own.
Tested malta_defconfig and maltael_defconfig to work again in Qemu.
What about the other architectures which can use the driver?
As for testing, I compile tested an ARM board, edminiv2, successfully, but don't know how to invoke qemu for it. As for running the tests, my setup is not capable of running the tests since they seem to be for linux only.
Does the automatic testing check if disks are indeed found correctly and checking reading or writing to a disk image?
With regards, Reinoud
On 24.02.21 22:55, Reinoud Zandijk wrote:
Dear Heinrich,
On Wed, Feb 24, 2021 at 06:57:08PM +0100, Heinrich Schuchardt wrote:
On 24.02.21 17:44, Reinoud Zandijk wrote:
Fixes IDE issues found on the Malta board under Qemu:
- DMA implied commands were sent to the controller in stead of the PIO
variants. The rest of the code is DMA free and written for PIO operation.
- direct pointer access was used to read and write the registers instead
of the inb/inw/outb/outw functions/macros. Registers don't have to be memory mapped and ATA_CURR_BASE() does not have to return an offset from address zero.
- Endian isues in ide_ident() and reading/writing data in general. Names
were corrupted and sizes misreported.
It is preferable to have each issue fixed in a separate patch.
They are related; 2 and 3 are really one and 1 could in theory be separate but it won't work on its own.
Thanks for the background.
Tested malta_defconfig and maltael_defconfig to work again in Qemu.
What about the other architectures which can use the driver?
As for testing, I compile tested an ARM board, edminiv2, successfully, but don't know how to invoke qemu for it. As for running the tests, my setup is not capable of running the tests since they seem to be for linux only.
Does the automatic testing check if disks are indeed found correctly and checking reading or writing to a disk image?
qemu-x86_64_defconfig and qemu-x86_defconfig have CONFIG_CMD_IDE=y.
Actually without your patch qemu-x86 cannot read the partition table from an IDE device. With your patch it works fine.
This was my QEMU command:
qemu-system-i386 -bios u-boot.rom -nographic sct-i386.img
Tested-by: Heinrich Schuchardt xypron.glpk@gmx.de
On Wed, Feb 24, 2021 at 05:44:42PM +0100, Reinoud Zandijk wrote:
Fixes IDE issues found on the Malta board under Qemu:
- DMA implied commands were sent to the controller in stead of the PIO
variants. The rest of the code is DMA free and written for PIO operation.
- direct pointer access was used to read and write the registers instead
of the inb/inw/outb/outw functions/macros. Registers don't have to be memory mapped and ATA_CURR_BASE() does not have to return an offset from address zero.
- Endian isues in ide_ident() and reading/writing data in general. Names
were corrupted and sizes misreported.
Tested malta_defconfig and maltael_defconfig to work again in Qemu.
Signed-off-by: Reinoud Zandijk reinoud@NetBSD.org Tested-by: Heinrich Schuchardt xypron.glpk@gmx.de
Applied to u-boot/master, thanks!
participants (3)
-
Heinrich Schuchardt -
Reinoud Zandijk -
Tom Rini