[PATCH v2 0/2] Fix MIPS/Malta target and its IDE work

[PATCH v2 0/2] Fix MIPS/Malta target and its IDE work

[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. It could also be a toolchain dependency.

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.

Signed-off-by: Reinoud Zandijk <reinoud@NetBSD.org>


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         | 149 +++++++++++-------------------------
 include/ata.h               |   2 +-
 6 files changed, 48 insertions(+), 107 deletions(-)

-- 
2.29.2

[PATCH v2 1/2] Re-embed the FDTs for the Malta targets.

[Reinoud Zandijk]

Re-enable FDT inclusion into the Malta u-boot binary to make them boot
again. This could indicate an unwanted toolchain version dependency.

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
-- 
2.29.2

[PATCH v2 2/2] Fix IDE commands issued, fix endian issues, fix non MMIO

[Reinoud Zandijk]

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.

With the fixes, malta_defconfig and maltael_defconfig work again in Qemu.


Signed-off-by: Reinoud Zandijk <reinoud@NetBSD.org>

---
 drivers/block/ide.c | 149 +++++++++++++-------------------------------
 include/ata.h       |   2 +-
 2 files changed, 44 insertions(+), 107 deletions(-)

diff --git a/drivers/block/ide.c b/drivers/block/ide.c
index 43a0776099..0d1ad33125 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,26 +633,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 +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..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 */
-- 
2.29.2

[PATCH v2 2/2] Fix IDE commands issued, fix endian issues, fix non MMIO

[Heinrich Schuchardt]

On 2/22/21 10:20 PM, Reinoud Zandijk wrote:
> 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.
>
> With the fixes, malta_defconfig and maltael_defconfig work again in Qemu.
>
>
> Signed-off-by: Reinoud Zandijk <reinoud@NetBSD.org>
>
> ---
>   drivers/block/ide.c | 149 +++++++++++++-------------------------------
>   include/ata.h       |   2 +-
>   2 files changed, 44 insertions(+), 107 deletions(-)
>
> diff --git a/drivers/block/ide.c b/drivers/block/ide.c
> index 43a0776099..0d1ad33125 100644
> --- a/drivers/block/ide.c
> +++ b/drivers/block/ide.c
> @@ -130,56 +130,40 @@ OUT:
>    * ATAPI Support
>    */
>
> -#if defined(CONFIG_IDE_SWAP_IO)

edminiv2_defconfig does not build with this change.

>   /* 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);

Why would you print an u64 as %lx and not as %llx?

>
>   	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);

ditto

Best regards

Heinrich

>
>   	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,26 +633,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 +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..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 */
>

[PATCH v2 1/2] Re-embed the FDTs for the Malta targets.

[Tom Rini]

On Mon, Feb 22, 2021 at 10:20:12PM +0100, Reinoud Zandijk wrote:

> Re-enable FDT inclusion into the Malta u-boot binary to make them boot
> again. This could indicate an unwanted toolchain version dependency.
> 
> Signed-off-by: Reinoud Zandijk <reinoud@NetBSD.org>

It would probably help to see how you're invoking qemu without this
change such that it fails.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210222/5b0e12e1/attachment.sig>

[PATCH v2 1/2] Re-embed the FDTs for the Malta targets.

[Simon Glass]

Hi Reinoud,

On Mon, 22 Feb 2021 at 16:20, Reinoud Zandijk <reinoud@netbsd.org> wrote:
>
> Re-enable FDT inclusion into the Malta u-boot binary to make them boot
> again. This could indicate an unwanted toolchain version dependency.
>
> 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(+)
>

As was mentioned by someone else we should not have this in mainline.
The commit that 'fixed' it to use OF_SEPARATE was submitted two years
ago, so this one seems to be a revert of that.

Given the time elapsed I think this should be figured out, not papered
over. Perhaps Daniel knows what has changed?

Regards,
Simon

[PATCH v2 1/2] Re-embed the FDTs for the Malta targets.

[Reinoud Zandijk]

On Mon, Feb 22, 2021 at 06:04:03PM -0500, Tom Rini wrote:
> On Mon, Feb 22, 2021 at 10:20:12PM +0100, Reinoud Zandijk wrote:
> 
> > Re-enable FDT inclusion into the Malta u-boot binary to make them boot
> > again. This could indicate an unwanted toolchain version dependency.
> > 
> > Signed-off-by: Reinoud Zandijk <reinoud@NetBSD.org>
> 
> It would probably help to see how you're invoking qemu without this
> change such that it fails.  Thanks!

My bad! I must have issued it wrong or picked the wrong u-boot.bin. Removing
the patch and invoking it as

qemu-system-mips -snapshot -M malta -m 256M -s -S -bios
  /usr/sources/u-boot/u-boot.bin -serial
  mon:telnet:127.0.0.1:2223,server,wait -nographic -hda
  work/disk-mips64-malta.fs -net nic -net tap,ifname=tap0,script=no

works OK! this was a red herring; i'll retract it. I must have used the wrong
u-boot result image without the fdt all this time :-/ DOH!

Thanks for pointing it out.
Reinoud

[PATCH v2 2/2] Fix IDE commands issued, fix endian issues, fix non MMIO

[Reinoud Zandijk]

On Mon, Feb 22, 2021 at 10:48:42PM +0100, Heinrich Schuchardt wrote:
> On 2/22/21 10:20 PM, Reinoud Zandijk wrote:
> > 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.
...
> > ---
> >   drivers/block/ide.c | 149 +++++++++++++-------------------------------
> >   include/ata.h       |   2 +-
> >   2 files changed, 44 insertions(+), 107 deletions(-)
> > 
> > diff --git a/drivers/block/ide.c b/drivers/block/ide.c
> > index 43a0776099..0d1ad33125 100644
> > --- a/drivers/block/ide.c
> > +++ b/drivers/block/ide.c
> > @@ -130,56 +130,40 @@ OUT:
> >    * ATAPI Support
> >    */
> > 
> > -#if defined(CONFIG_IDE_SWAP_IO)
> 
> edminiv2_defconfig does not build with this change.

Now thats odd! The inw() and outw() macro's seem to have different definitions
depending on what architecture you're compiling for!

For at least MIPS and m68k , inw() is defined as inw(long unsigned int) but
for ARM its defined as inw(short *) !

Now what should it be?

Also I could use in_le16() and out_le16() but will they be defined for all
architectures? Or should not supporting it be considered a bug on their side?

Using uintptr_t seems to satisfy both as its guaranteed to be representing a
ptr and an int and GCC isn't even complaining anymore on either platforms.
I'll use this one in my next patch version

Thanks for the feedback,
Reinoud

[PATCH v2 1/2] Re-embed the FDTs for the Malta targets.

[Simon Glass]

Hi Reinoud,

On Tue, 23 Feb 2021 at 09:37, Reinoud Zandijk <reinoud@netbsd.org> wrote:
>
> On Mon, Feb 22, 2021 at 06:04:03PM -0500, Tom Rini wrote:
> > On Mon, Feb 22, 2021 at 10:20:12PM +0100, Reinoud Zandijk wrote:
> >
> > > Re-enable FDT inclusion into the Malta u-boot binary to make them boot
> > > again. This could indicate an unwanted toolchain version dependency.
> > >
> > > Signed-off-by: Reinoud Zandijk <reinoud@NetBSD.org>
> >
> > It would probably help to see how you're invoking qemu without this
> > change such that it fails.  Thanks!
>
> My bad! I must have issued it wrong or picked the wrong u-boot.bin. Removing
> the patch and invoking it as
>
> qemu-system-mips -snapshot -M malta -m 256M -s -S -bios
>   /usr/sources/u-boot/u-boot.bin -serial
>   mon:telnet:127.0.0.1:2223,server,wait -nographic -hda
>   work/disk-mips64-malta.fs -net nic -net tap,ifname=tap0,script=no
>
> works OK! this was a red herring; i'll retract it. I must have used the wrong
> u-boot result image without the fdt all this time :-/ DOH!

OK great! It sounds like there might be some missing docs here, to
prevent others hitting this problem.

>
> Thanks for pointing it out.
> Reinoud
>

Regards,
Simon