[PATCH 0/2] Fix MIPS/Malta target and its IDE work
Patch 0001 re-enables FDT inclusion into the u-boot binary to make them boot again. The code might not have adjusted well enough in the past to handle the separate one.
Patch 0002 fixes IDE issues found on the Malta board:
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.
With the fixes, malta_defconfig and maltael_defconfig work again in Qemu.
Reinoud Zandijk (2): Re-embed the FDTs for the Malta targets. Fix IDE commands issued, fix endian issues, fix non MMIO
configs/malta64_defconfig | 1 + configs/malta64el_defconfig | 1 + configs/malta_defconfig | 1 + configs/maltael_defconfig | 1 + drivers/block/ide.c | 143 ++++++++++-------------------------- include/ata.h | 3 +- 6 files changed, 46 insertions(+), 104 deletions(-)
Signed-off-by: Reinoud Zandijk reinoud@NetBSD.org
--- configs/malta64_defconfig | 1 + configs/malta64el_defconfig | 1 + configs/malta_defconfig | 1 + configs/maltael_defconfig | 1 + 4 files changed, 4 insertions(+)
diff --git a/configs/malta64_defconfig b/configs/malta64_defconfig index 878dc6ee05..106c890faa 100644 --- a/configs/malta64_defconfig +++ b/configs/malta64_defconfig @@ -21,6 +21,7 @@ CONFIG_CMD_DHCP=y CONFIG_CMD_PING=y CONFIG_CMD_DATE=y # CONFIG_ISO_PARTITION is not set +CONFIG_OF_EMBED=y CONFIG_ENV_IS_IN_FLASH=y CONFIG_ENV_ADDR=0xFFFFFFFFBE3E0000 CONFIG_MTD_NOR_FLASH=y diff --git a/configs/malta64el_defconfig b/configs/malta64el_defconfig index 7dfe67355f..e61d61b26b 100644 --- a/configs/malta64el_defconfig +++ b/configs/malta64el_defconfig @@ -23,6 +23,7 @@ CONFIG_CMD_DHCP=y CONFIG_CMD_PING=y CONFIG_CMD_DATE=y # CONFIG_ISO_PARTITION is not set +CONFIG_OF_EMBED=y CONFIG_ENV_IS_IN_FLASH=y CONFIG_ENV_ADDR=0xFFFFFFFFBE3E0000 CONFIG_MTD_NOR_FLASH=y diff --git a/configs/malta_defconfig b/configs/malta_defconfig index 304f2198ba..c1dfa357d2 100644 --- a/configs/malta_defconfig +++ b/configs/malta_defconfig @@ -20,6 +20,7 @@ CONFIG_CMD_DHCP=y CONFIG_CMD_PING=y CONFIG_CMD_DATE=y # CONFIG_ISO_PARTITION is not set +CONFIG_OF_EMBED=y CONFIG_ENV_IS_IN_FLASH=y CONFIG_ENV_ADDR=0xBE3E0000 CONFIG_MTD_NOR_FLASH=y diff --git a/configs/maltael_defconfig b/configs/maltael_defconfig index 7436e4e943..0a8badcb84 100644 --- a/configs/maltael_defconfig +++ b/configs/maltael_defconfig @@ -22,6 +22,7 @@ CONFIG_CMD_DHCP=y CONFIG_CMD_PING=y CONFIG_CMD_DATE=y # CONFIG_ISO_PARTITION is not set +CONFIG_OF_EMBED=y CONFIG_ENV_IS_IN_FLASH=y CONFIG_ENV_ADDR=0xBE3E0000 CONFIG_MTD_NOR_FLASH=y
On 2/22/21 6:05 PM, Reinoud Zandijk wrote:
Please, provide a commit message and a Signed-by line.
configs/malta64_defconfig | 1 + configs/malta64el_defconfig | 1 + configs/malta_defconfig | 1 + configs/maltael_defconfig | 1 + 4 files changed, 4 insertions(+)
diff --git a/configs/malta64_defconfig b/configs/malta64_defconfig index 878dc6ee05..106c890faa 100644 --- a/configs/malta64_defconfig +++ b/configs/malta64_defconfig @@ -21,6 +21,7 @@ CONFIG_CMD_DHCP=y CONFIG_CMD_PING=y CONFIG_CMD_DATE=y # CONFIG_ISO_PARTITION is not set +CONFIG_OF_EMBED=y CONFIG_ENV_IS_IN_FLASH=y CONFIG_ENV_ADDR=0xFFFFFFFFBE3E0000 CONFIG_MTD_NOR_FLASH=y diff --git a/configs/malta64el_defconfig b/configs/malta64el_defconfig index 7dfe67355f..e61d61b26b 100644 --- a/configs/malta64el_defconfig +++ b/configs/malta64el_defconfig @@ -23,6 +23,7 @@ CONFIG_CMD_DHCP=y CONFIG_CMD_PING=y CONFIG_CMD_DATE=y # CONFIG_ISO_PARTITION is not set +CONFIG_OF_EMBED=y CONFIG_ENV_IS_IN_FLASH=y CONFIG_ENV_ADDR=0xFFFFFFFFBE3E0000 CONFIG_MTD_NOR_FLASH=y diff --git a/configs/malta_defconfig b/configs/malta_defconfig index 304f2198ba..c1dfa357d2 100644 --- a/configs/malta_defconfig +++ b/configs/malta_defconfig @@ -20,6 +20,7 @@ CONFIG_CMD_DHCP=y CONFIG_CMD_PING=y CONFIG_CMD_DATE=y # CONFIG_ISO_PARTITION is not set +CONFIG_OF_EMBED=y
This should not be used in a defconfig.
dts/Kconfig:83:
"Boards in the mainline U-Boot tree should not use it."
Best regards
Heinrich
CONFIG_ENV_IS_IN_FLASH=y CONFIG_ENV_ADDR=0xBE3E0000 CONFIG_MTD_NOR_FLASH=y diff --git a/configs/maltael_defconfig b/configs/maltael_defconfig index 7436e4e943..0a8badcb84 100644 --- a/configs/maltael_defconfig +++ b/configs/maltael_defconfig @@ -22,6 +22,7 @@ CONFIG_CMD_DHCP=y CONFIG_CMD_PING=y CONFIG_CMD_DATE=y # CONFIG_ISO_PARTITION is not set +CONFIG_OF_EMBED=y CONFIG_ENV_IS_IN_FLASH=y CONFIG_ENV_ADDR=0xBE3E0000 CONFIG_MTD_NOR_FLASH=y
--- drivers/block/ide.c | 143 +++++++++++++------------------------------- include/ata.h | 3 +- 2 files changed, 42 insertions(+), 104 deletions(-)
diff --git a/drivers/block/ide.c b/drivers/block/ide.c index 43a0776099..fa3481e936 100644 --- a/drivers/block/ide.c +++ b/drivers/block/ide.c @@ -130,56 +130,40 @@ 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) { ushort *dbuf; - volatile ushort *pbuf; + u64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
- 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); + (unsigned long) paddr);
while (shorts--) { EIEIO; - *pbuf = *dbuf++; + outw(paddr, cpu_to_le16(*dbuf++)); } }
__weak void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts) { ushort *dbuf; - volatile ushort *pbuf; + u64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
- 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); + (unsigned long) 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 +620,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,23 +633,19 @@ 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] | + 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 +813,60 @@ 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); + u64 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); + (unsigned long) 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 ushort *dbuf; - volatile ushort *pbuf; + u64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
- pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); dbuf = (ushort *)sect_buf; while (words--) { EIEIO; - *pbuf = *dbuf++; + outw(paddr, cpu_to_le16(*dbuf++)); EIEIO; - *pbuf = *dbuf++; + outw(paddr, cpu_to_le16(*dbuf++)); } +#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 ushort *dbuf; - volatile ushort *pbuf; + u64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
- 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 %lx\n", (unsigned long) 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 +956,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 +1053,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..7c95cfdced 100644 --- a/include/ata.h +++ b/include/ata.h @@ -134,7 +134,8 @@ 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 int lba_capacity; /--* total number of sectors */ + unsigned short lba_capacity[2]; /* 2 16bit values containing lba 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 2/22/21 6:05 PM, Reinoud Zandijk wrote:
A patch without commit message will not be accepted.
Please, run scripts/checkpatch.pl on your patch:
ERROR: Missing Signed-off-by: line(s)
drivers/block/ide.c | 143 +++++++++++++------------------------------- include/ata.h | 3 +- 2 files changed, 42 insertions(+), 104 deletions(-)
diff --git a/drivers/block/ide.c b/drivers/block/ide.c index 43a0776099..fa3481e936 100644 --- a/drivers/block/ide.c
Your patch is mal-formed:
ERROR: Avoid using diff content in the commit message - patch(1) might not work #126: --- a/drivers/block/ide.c
+++ b/drivers/block/ide.c @@ -130,56 +130,40 @@ OUT:
- ATAPI Support
*/
-#if defined(CONFIG_IDE_SWAP_IO)
As far as possible replace #if by if.
/* 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) { ushort *dbuf;
- volatile ushort *pbuf;
- u64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
ERROR: DOS line endings #139: FILE: drivers/block/ide.c:138: +^Iu64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);^M$
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);
(unsigned long) paddr);while (shorts--) { EIEIO;
*pbuf = *dbuf++;
outw(paddr, cpu_to_le16(*dbuf++));} }
__weak void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts) { ushort *dbuf;
- volatile ushort *pbuf;
- u64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
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);
(unsigned long) 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 +620,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,23 +633,19 @@ 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] |
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 +813,60 @@ 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);
u64 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);
(unsigned long) 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 ushort *dbuf;
- volatile ushort *pbuf;
- u64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
- pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); dbuf = (ushort *)sect_buf; while (words--) { EIEIO;
*pbuf = *dbuf++;
outw(paddr, cpu_to_le16(*dbuf++));
*pbuf = *dbuf++;
outw(paddr, cpu_to_le16(*dbuf++));+#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 ushort *dbuf;
- volatile ushort *pbuf;
- u64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);
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 %lx\n", (unsigned long) 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 +956,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 +1053,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..7c95cfdced 100644 --- a/include/ata.h +++ b/include/ata.h @@ -134,7 +134,8 @@ 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 int lba_capacity; /--* total number of sectors */
Please, remove replaced code completely.
- unsigned short lba_capacity[2]; /* 2 16bit values containing lba total number of sectors */
WARNING: line length of 101 exceeds 100 columns #400: FILE: include/ata.h:138: + unsigned short lba_capacity[2]; /* 2 16bit values containing lba total number of sectors */
Best regards
Heinrich
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 */
Am Montag, den 22.02.2021, 18:05 +0100 schrieb Reinoud Zandijk:
Patch 0001 re-enables FDT inclusion into the u-boot binary to make them boot again. The code might not have adjusted well enough in the past to handle the separate one.
what exactly is the issue? Do you see it just on real hardware?
Booting all Malta variants in Qemu is contained in official U-Boot CI and showed no boot failures until now. Unfortuneately I don't have Malta hardware myself for testing.
Also CONFIG_OF_EMBED is just a debug option and should be avoided for production builds. So if there is a real problem, I would prefer to rather fix that instead of enabling CONFIG_OF_EMBED.
Patch 0002 fixes IDE issues found on the Malta board:
- 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.
With the fixes, malta_defconfig and maltael_defconfig work again in Qemu.
Reinoud Zandijk (2): Re-embed the FDTs for the Malta targets. Fix IDE commands issued, fix endian issues, fix non MMIO
configs/malta64_defconfig | 1 + configs/malta64el_defconfig | 1 + configs/malta_defconfig | 1 + configs/maltael_defconfig | 1 + drivers/block/ide.c | 143 ++++++++++----------------------
include/ata.h | 3 +- 6 files changed, 46 insertions(+), 104 deletions(-)
Signed-off-by: Reinoud Zandijk reinoud@NetBSD.org
Hi Daniel,
On Mon, Feb 22, 2021 at 07:23:26PM +0100, Daniel Schwierzeck wrote:
Am Montag, den 22.02.2021, 18:05 +0100 schrieb Reinoud Zandijk:
Patch 0001 re-enables FDT inclusion into the u-boot binary to make them boot again. The code might not have adjusted well enough in the past to handle the separate one.
what exactly is the issue? Do you see it just on real hardware?
Booting all Malta variants in Qemu is contained in official U-Boot CI and showed no boot failures until now. Unfortuneately I don't have Malta hardware myself for testing.
If I remove it, the machine just spins in Qemu, no output, nothing. If I add it, it works fine again. I found out by bisecting. I have no idea why the tests aren't picking this up. Could it be a qemu/gcc/binutils combination issue? A symbol not set as expected?
qemu 5.1.0 gcc 8.3.0 binutils 2.32
Also CONFIG_OF_EMBED is just a debug option and should be avoided for production builds. So if there is a real problem, I would prefer to rather fix that instead of enabling CONFIG_OF_EMBED.
Thats what I already stated; Its a provisionary fix until the real issue is tacked. Its now at least known.
With regards, Reinoud
Am Montag, den 22.02.2021, 20:56 +0100 schrieb Reinoud Zandijk:
Hi Daniel,
On Mon, Feb 22, 2021 at 07:23:26PM +0100, Daniel Schwierzeck wrote:
Am Montag, den 22.02.2021, 18:05 +0100 schrieb Reinoud Zandijk:
Patch 0001 re-enables FDT inclusion into the u-boot binary to make them boot again. The code might not have adjusted well enough in the past to handle the separate one.
what exactly is the issue? Do you see it just on real hardware?
Booting all Malta variants in Qemu is contained in official U-Boot CI and showed no boot failures until now. Unfortuneately I don't have Malta hardware myself for testing.
If I remove it, the machine just spins in Qemu, no output, nothing. If I add it, it works fine again. I found out by bisecting. I have no idea why the tests aren't picking this up. Could it be a qemu/gcc/binutils combination issue? A symbol not set as expected?
qemu 5.1.0 gcc 8.3.0 binutils 2.32
which board config did you try exactly? malta or maltael?
For maltael and malta64el an extra U-Boot binary named u-boot-swap.bin is generated which is booted with Qemu. See also [1] respectively all conf.malta*_qemu configs for working Qemu configurations for all Malta variants.
The reason for that extra image is that the original MIPS YAMON loader builds a combined EB/EL image and links it to EB. With some assembly magic the start code determines immediately after the reset vector which endianess the CPU is running and branches to the according EB or LE image. This is replicated in the Qemu Malta machine implementation. The Malta EL machine code excepts a bootloader in EB format and swaps the code back to EL before executing it. To workaround this, we need to feed this u-boot-swap.bin to Qemu.
[1] https://gitlab.denx.de/u-boot/u-boot-test-hooks/-blob/master/bin/travis-ci/c...
Hi Daniel,
On Tue, Feb 23, 2021 at 01:03:05AM +0100, Daniel Schwierzeck wrote:
Am Montag, den 22.02.2021, 20:56 +0100 schrieb Reinoud Zandijk:
If I remove it, the machine just spins in Qemu, no output, nothing. If I add it, it works fine again. I found out by bisecting. I have no idea why the tests aren't picking this up. Could it be a qemu/gcc/binutils combination issue? A symbol not set as expected?
qemu 5.1.0 gcc 8.3.0 binutils 2.32
which board config did you try exactly? malta or maltael?
Both malta and maltael have the same behaviour. And yeah, for maltael i needed the u-boot-swap.bin indeed! That was not that obvious at first but trial and error showed it.
How are the tests performed? Are the actual u-boot images passed as compiled with `-bios' to qemu and that alone? Or are the tests also sneaking in the FDT to qemu? By f.e. appending them? Could the CONFIG_SYS_MALLOC_CLEAR_ON_INIT warning/error prevented a good build? I also get that with edminib2_defconfig so I assumed some work is still in progress on that.
[1] https://gitlab.denx.de/u-boot/u-boot-test-hooks/-blob/master/bin/travis-ci/c...
This is behind a login?
Reinoud
On Tue, Feb 23, 2021 at 03:19:06PM +0100, Reinoud Zandijk wrote:
Hi Daniel,
On Tue, Feb 23, 2021 at 01:03:05AM +0100, Daniel Schwierzeck wrote:
Am Montag, den 22.02.2021, 20:56 +0100 schrieb Reinoud Zandijk:
If I remove it, the machine just spins in Qemu, no output, nothing. If I add it, it works fine again. I found out by bisecting. I have no idea why the tests aren't picking this up. Could it be a qemu/gcc/binutils combination issue? A symbol not set as expected?
qemu 5.1.0 gcc 8.3.0 binutils 2.32
which board config did you try exactly? malta or maltael?
Both malta and maltael have the same behaviour. And yeah, for maltael i needed the u-boot-swap.bin indeed! That was not that obvious at first but trial and error showed it.
How are the tests performed? Are the actual u-boot images passed as compiled with `-bios' to qemu and that alone? Or are the tests also sneaking in the FDT to qemu? By f.e. appending them? Could the CONFIG_SYS_MALLOC_CLEAR_ON_INIT warning/error prevented a good build? I also get that with edminib2_defconfig so I assumed some work is still in progress on that.
[1] https://gitlab.denx.de/u-boot/u-boot-test-hooks/-blob/master/bin/travis-ci/c...
This is behind a login?
Unintentionally so, yes. Fixing this now.
On Tue, Feb 23, 2021 at 09:26:56AM -0500, Tom Rini wrote:
On Tue, Feb 23, 2021 at 03:19:06PM +0100, Reinoud Zandijk wrote:
Hi Daniel,
On Tue, Feb 23, 2021 at 01:03:05AM +0100, Daniel Schwierzeck wrote:
Am Montag, den 22.02.2021, 20:56 +0100 schrieb Reinoud Zandijk:
If I remove it, the machine just spins in Qemu, no output, nothing. If I add it, it works fine again. I found out by bisecting. I have no idea why the tests aren't picking this up. Could it be a qemu/gcc/binutils combination issue? A symbol not set as expected?
qemu 5.1.0 gcc 8.3.0 binutils 2.32
which board config did you try exactly? malta or maltael?
Both malta and maltael have the same behaviour. And yeah, for maltael i needed the u-boot-swap.bin indeed! That was not that obvious at first but trial and error showed it.
How are the tests performed? Are the actual u-boot images passed as compiled with `-bios' to qemu and that alone? Or are the tests also sneaking in the FDT to qemu? By f.e. appending them? Could the CONFIG_SYS_MALLOC_CLEAR_ON_INIT warning/error prevented a good build? I also get that with edminib2_defconfig so I assumed some work is still in progress on that.
[1] https://gitlab.denx.de/u-boot/u-boot-test-hooks/-blob/master/bin/travis-ci/c...
This is behind a login?
Unintentionally so, yes. Fixing this now.
Ah! A typo that gets fixed up automatically if logged in, but not otherwise: https://gitlab.denx.de/u-boot/u-boot-test-hooks/-/blob/master/bin/travis-ci/... is right (note the '/' after '-' before 'blob').
Am Dienstag, den 23.02.2021, 15:19 +0100 schrieb Reinoud Zandijk:
Hi Daniel,
On Tue, Feb 23, 2021 at 01:03:05AM +0100, Daniel Schwierzeck wrote:
Am Montag, den 22.02.2021, 20:56 +0100 schrieb Reinoud Zandijk:
If I remove it, the machine just spins in Qemu, no output, nothing. If I add it, it works fine again. I found out by bisecting. I have no idea why the tests aren't picking this up. Could it be a qemu/gcc/binutils combination issue? A symbol not set as expected?
qemu 5.1.0 gcc 8.3.0 binutils 2.32
which board config did you try exactly? malta or maltael?
Both malta and maltael have the same behaviour. And yeah, for maltael i needed the u-boot-swap.bin indeed! That was not that obvious at first but trial and error showed it.
How are the tests performed? Are the actual u-boot images passed as compiled with `-bios' to qemu and that alone? Or are the tests also sneaking in the FDT to qemu? By f.e. appending them?
we use the images as built by the default configs. We don't use "-bios" but "-drive if=pflash,file=${U_BOOT_BUILD_DIR}/flash.img,format=raw" to emulate NOR flash and to be closer to hardware.
flash.img is created with that script: https://gitlab.denx.de/u-boot/u-boot-test-hooks/-/blob/master/bin/flash.qemu...
On 2/23/21 7:06 PM, Daniel Schwierzeck wrote:
Am Dienstag, den 23.02.2021, 15:19 +0100 schrieb Reinoud Zandijk:
Hi Daniel,
On Tue, Feb 23, 2021 at 01:03:05AM +0100, Daniel Schwierzeck wrote:
Am Montag, den 22.02.2021, 20:56 +0100 schrieb Reinoud Zandijk:
If I remove it, the machine just spins in Qemu, no output, nothing. If I add it, it works fine again. I found out by bisecting. I have no idea why the tests aren't picking this up. Could it be a qemu/gcc/binutils combination issue? A symbol not set as expected?
qemu 5.1.0 gcc 8.3.0 binutils 2.32
which board config did you try exactly? malta or maltael?
Both malta and maltael have the same behaviour. And yeah, for maltael i needed the u-boot-swap.bin indeed! That was not that obvious at first but trial and error showed it.
How are the tests performed? Are the actual u-boot images passed as compiled with `-bios' to qemu and that alone? Or are the tests also sneaking in the FDT to qemu? By f.e. appending them?
we use the images as built by the default configs. We don't use "-bios" but "-drive if=pflash,file=${U_BOOT_BUILD_DIR}/flash.img,format=raw" to emulate NOR flash and to be closer to hardware.
flash.img is created with that script: https://gitlab.denx.de/u-boot/u-boot-test-hooks/-/blob/master/bin/flash.qemu...
Hello Daniel,
thanks for pointing out how the Gitlab CI test is run.
This looks a bit different to doc/board/emulation/qemu-mips.rst where '-pflash flash' is used as argument.
Do you want to update the man-page to use -drive instead of -pflash?
Best regards
Heinrich
participants (4)
-
Daniel Schwierzeck -
Heinrich Schuchardt -
Reinoud Zandijk -
Tom Rini