[patch 1/1] SCSI: improve endian handling in LSI fusion firmware mpt_downloadboot

[patch 1/1] SCSI: improve endian handling in LSI fusion firmware mpt_downloadboot

[Erik Habbinga]

The mpt_downloadboot function in drivers/message/fusion/mptbase.c doesn't work correctly on big endian systems (powerpc in my case).
I've added appropriate le32_to_cpu calls to correctly translate little endian firmware file data into cpu endian format before
getting written to little endian PCI registers.  This patch has been tested successfully on a powerpc target and an Intel target.

Signed-off-by: Erik Habbinga <erikhabbinga@inphase-tech.com>

--- a/drivers/message/fusion/mptbase.c.orig	2006-08-23 15:16:33.000000000 -0600
+++ b/drivers/message/fusion/mptbase.c	2006-08-30 10:28:39.000000000 -0600
@@ -2915,7 +2915,7 @@
 	u32 			 ioc_state=0;
 
 	ddlprintk((MYIOC_s_INFO_FMT "downloadboot: fw size 0x%x (%d), FW Ptr %p\n",
-				ioc->name, pFwHeader->ImageSize, pFwHeader->ImageSize, pFwHeader));
+				ioc->name, le32_to_cpu(pFwHeader->ImageSize), le32_to_cpu(pFwHeader->ImageSize), pFwHeader));
 
 	CHIPREG_WRITE32(&ioc->chip->WriteSequence, 0xFF);
 	CHIPREG_WRITE32(&ioc->chip->WriteSequence, MPI_WRSEQ_1ST_KEY_VALUE);
@@ -2968,7 +2968,7 @@
 	/* Set the DiagRwEn and Disable ARM bits */
 	CHIPREG_WRITE32(&ioc->chip->Diagnostic, (MPI_DIAG_RW_ENABLE | MPI_DIAG_DISABLE_ARM));
 
-	fwSize = (pFwHeader->ImageSize + 3)/4;
+	fwSize = (le32_to_cpu(pFwHeader->ImageSize) + 3)/4;
 	ptrFw = (u32 *) pFwHeader;
 
 	/* Write the LoadStartAddress to the DiagRw Address Register
@@ -2977,23 +2977,23 @@
 	if (ioc->errata_flag_1064)
 		pci_enable_io_access(ioc->pcidev);
 
-	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, pFwHeader->LoadStartAddress);
+	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, le32_to_cpu(pFwHeader->LoadStartAddress));
 	ddlprintk((MYIOC_s_INFO_FMT "LoadStart addr written 0x%x \n",
-		ioc->name, pFwHeader->LoadStartAddress));
+		ioc->name, le32_to_cpu(pFwHeader->LoadStartAddress)));
 
 	ddlprintk((MYIOC_s_INFO_FMT "Write FW Image: 0x%x bytes @ %p\n",
 				ioc->name, fwSize*4, ptrFw));
 	while (fwSize--) {
-		CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, *ptrFw++);
+		CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, le32_to_cpu(*ptrFw++));
 	}
 
-	nextImage = pFwHeader->NextImageHeaderOffset;
+	nextImage = le32_to_cpu(pFwHeader->NextImageHeaderOffset);
 	while (nextImage) {
 		pExtImage = (MpiExtImageHeader_t *) ((char *)pFwHeader + nextImage);
 
-		load_addr = pExtImage->LoadStartAddress;
+		load_addr = le32_to_cpu(pExtImage->LoadStartAddress);
 
-		fwSize = (pExtImage->ImageSize + 3) >> 2;
+		fwSize = (le32_to_cpu(pExtImage->ImageSize) + 3) >> 2;
 		ptrFw = (u32 *)pExtImage;
 
 		ddlprintk((MYIOC_s_INFO_FMT "Write Ext Image: 0x%x (%d) bytes @ %p load_addr=%x\n",
@@ -3001,18 +3001,18 @@
 		CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, load_addr);
 
 		while (fwSize--) {
-			CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, *ptrFw++);
+			CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, le32_to_cpu(*ptrFw++));
 		}
-		nextImage = pExtImage->NextImageHeaderOffset;
+		nextImage = le32_to_cpu(pExtImage->NextImageHeaderOffset);
 	}
 
 	/* Write the IopResetVectorRegAddr */
-	ddlprintk((MYIOC_s_INFO_FMT "Write IopResetVector Addr=%x! \n", ioc->name, 	pFwHeader->IopResetRegAddr));
-	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, pFwHeader->IopResetRegAddr);
+	ddlprintk((MYIOC_s_INFO_FMT "Write IopResetVector Addr=%x! \n", ioc->name, 	le32_to_cpu(pFwHeader->IopResetRegAddr)));
+	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, le32_to_cpu(pFwHeader->IopResetRegAddr));
 
 	/* Write the IopResetVectorValue */
-	ddlprintk((MYIOC_s_INFO_FMT "Write IopResetVector Value=%x! \n", ioc->name, pFwHeader->IopResetVectorValue));
-	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, pFwHeader->IopResetVectorValue);
+	ddlprintk((MYIOC_s_INFO_FMT "Write IopResetVector Value=%x! \n", ioc->name, le32_to_cpu(pFwHeader->IopResetVectorValue)));
+	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, le32_to_cpu(pFwHeader->IopResetVectorValue));
 
 	/* Clear the internal flash bad bit - autoincrementing register,
 	 * so must do two writes.

Re: [patch 1/1] SCSI: improve endian handling in LSI fusion firmware mpt_downloadboot

[Andrew Morton]

On Wed, 30 Aug 2006 11:32:47 -0600
"Erik Habbinga" <erikhabbinga@inphase-tech.com> wrote:

> The mpt_downloadboot function in drivers/message/fusion/mptbase.c doesn't work correctly on big endian systems (powerpc in my case).
> I've added appropriate le32_to_cpu calls to correctly translate little endian firmware file data into cpu endian format before
> getting written to little endian PCI registers.  This patch has been tested successfully on a powerpc target and an Intel target.
> 
> Signed-off-by: Erik Habbinga <erikhabbinga@inphase-tech.com>
> 
> --- a/drivers/message/fusion/mptbase.c.orig	2006-08-23 15:16:33.000000000 -0600
> +++ b/drivers/message/fusion/mptbase.c	2006-08-30 10:28:39.000000000 -0600
> @@ -2915,7 +2915,7 @@
>  	u32 			 ioc_state=0;
>  
>  	ddlprintk((MYIOC_s_INFO_FMT "downloadboot: fw size 0x%x (%d), FW Ptr %p\n",
> -				ioc->name, pFwHeader->ImageSize, pFwHeader->ImageSize, pFwHeader));
> +				ioc->name, le32_to_cpu(pFwHeader->ImageSize), le32_to_cpu(pFwHeader->ImageSize), pFwHeader));
>  
>  	CHIPREG_WRITE32(&ioc->chip->WriteSequence, 0xFF);
>  	CHIPREG_WRITE32(&ioc->chip->WriteSequence, MPI_WRSEQ_1ST_KEY_VALUE);
> @@ -2968,7 +2968,7 @@
>  	/* Set the DiagRwEn and Disable ARM bits */
>  	CHIPREG_WRITE32(&ioc->chip->Diagnostic, (MPI_DIAG_RW_ENABLE | MPI_DIAG_DISABLE_ARM));
>  
> -	fwSize = (pFwHeader->ImageSize + 3)/4;
> +	fwSize = (le32_to_cpu(pFwHeader->ImageSize) + 3)/4;
>  	ptrFw = (u32 *) pFwHeader;
>  
>  	/* Write the LoadStartAddress to the DiagRw Address Register
> @@ -2977,23 +2977,23 @@
>  	if (ioc->errata_flag_1064)
>  		pci_enable_io_access(ioc->pcidev);
>  
> -	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, pFwHeader->LoadStartAddress);
> +	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, le32_to_cpu(pFwHeader->LoadStartAddress));
>  	ddlprintk((MYIOC_s_INFO_FMT "LoadStart addr written 0x%x \n",
> -		ioc->name, pFwHeader->LoadStartAddress));
> +		ioc->name, le32_to_cpu(pFwHeader->LoadStartAddress)));
>  
>  	ddlprintk((MYIOC_s_INFO_FMT "Write FW Image: 0x%x bytes @ %p\n",
>  				ioc->name, fwSize*4, ptrFw));
>  	while (fwSize--) {
> -		CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, *ptrFw++);
> +		CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, le32_to_cpu(*ptrFw++));

hm, I'd be a bit more comfortable if this didn't require that le32_to_cpu()
was sanely implemented.   include/linux/byteorder/swab.h has

#  define __swab32(x) \
(__builtin_constant_p((__u32)(x)) ? \
 ___swab32((x)) : \
 __fswab32((x)))

so if `x' is foo++, what value does `foo' end up with?  I guess it would be
pretty dumb if __builtin_constant_p(foo++) were to increment foo, but is
that guaranteed?

Anyway.  This is a moderately important fix, isn't it?  Eric (Moore), do
you want to proceed with this patch?

Thanks.

RE: [patch 1/1] SCSI: improve endian handling in LSI fusion firmware mpt_downloadboot

[Moore, Eric]

On Wednesday, August 30, 2006 11:33 AM, Erik Habbinga wrote: 

> The mpt_downloadboot function in 
> drivers/message/fusion/mptbase.c doesn't work correctly on 
> big endian systems (powerpc in my case).
> I've added appropriate le32_to_cpu calls to correctly 
> translate little endian firmware file data into cpu endian 
> format before
> getting written to little endian PCI registers.  This patch 
> has been tested successfully on a powerpc target and an Intel target.
> 

Rejected.

This will break all our customers on big-endian machines.

Our firmware is packaged in proper byte order on dword boundarys,
and doesn't need swapping at all.   Basically mpt_downloadboot, 
is reading from pFwHeader, and writing back out to doorbell
in proper byte order that doorbell expecting.  This code is working
for many LSI customers running on big-endian systems, such as pppc64.

Could you send your firmware image, copied to Stephen Shirron,
so he can determine if your firmware is properly packaged in correct
byte order?

Eric Moore
LSI Logic