[parisc-linux-cvs] 64-bit cleanup for the sim700 driver.

[Grant Grundler]

Ryan Bradetich wrote:
...
>  #if 0	/* test1_src/dst need to be in targdata if we ever use them */
>  	patch_add_32 (targdata->script, 0, test1_src, 
> -    		(u32)&(hostbus->test1_src));
> +    		low32(&(hostbus->test1_src));)
>  	patch_add_32 (targdata->script, 0, test1_dst, 
> -    		(u32)&(hostbus->test1_dst));
> +    		low32(&(hostbus->test1_dst)));
>  #endif

I was wondering how the ';)' above compiled and then noticed it wasn't.
It would be nice if it were correct though in case it gets copied to
another location and used.

...
>  	targdata->din_script[MAX_SG*2+1] =
> -		cpu_to_ncr32((u32)(targdata->ba->script + Ent_end_data_trans/4)
>   );
> +		cpu_to_ncr32(low32(targdata->ba->script + Ent_end_data_trans/4)
>   );

Some code uses sizeof(u32) and other '4'.
I can't say "sizeof(u32)" is more "correct" than hardcoding '4'.
My preference is just one of those forms be used though unless there
is a sematic difference (which doesn't seem to be the case).

...
> @@ -888,7 +887,7 @@ handle_phase_mismatch (struct Scsi_Host 
>      unsigned char sbcl;
>  
>      sbcl = NCR_read8(SBCL_REG) & SBCL_PHASE_MASK;
> -    index = (u32)((u32 *)(NCR_read32(DSP_REG)) - targdata->ba->script);
> +    index = low32((u32 *)(unsigned long)NCR_read32(DSP_REG) - targdata->ba->
>   script);

I'm abit confused by the casts. Is this meant?
	index = NCR_read32(DSP_REG) - low32(targdata->ba->script);

...
> @@ -1243,8 +1242,8 @@ sim700_intr_handle(int irq, void *dev_id
>  	    printk("scsi%d: Int %d, istat %02x, sstat0 %02x "
>  		"dstat %02x, dsp [%04x], target %x, dsps %08x\n",
>  		host->host_no, sim700_intrs, istat, sstat0, dstat,
> -		(u32 *)(NCR_read32(DSP_REG)) - targdata->ba->script,
> -		target, dsps);
> +		low32((u32 *)(unsigned long)NCR_read32(DSP_REG) - 
> +	  	(unsigned long)targdata->ba->script), target, dsps);

Again, similar construct with casts.

...
> @@ -1255,7 +1254,7 @@ sim700_intr_handle(int irq, void *dev_id
>  	    resume_offset = Ent_reselect;
>  	}
>  	else if (hostdata->chip == 700 && (dstat & DSTAT_ABRT)) {
> -	    u32 offset = (dsp - (u32)targdata->ba->script)/4;
> +	    u32 offset = (dsp - low32(targdata->ba->script)/4);

This looks wrong to me. Is it? (parenthesis moved)

Ok...had enough for now...

grant

Grant Grundler
parisc-linux {PCI|IOMMU|SMP} hacker
+1.408.447.7253