lkml.org 
[lkml]   [2003]   [Dec]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2.6.0] megaraid 64bit fix/cleanup (AMD64)
Brad House wrote:
> Hmm, I don't think this driver is as complex as others may
> be to port. But then again, I'm probably wrong b/c I'm mainly
> a userland guy, not a kernel guy :/
> But, nonetheless, I've made some changes:

The only thing that matters is the hardware s/g list capabilities.
Driver complexity is irrelevant.


>>> /* Calculate Scatter-Gather info */
>>> mbox->m_out.numsgelements = mega_build_sglist(adapter, scb,
>>>- (u32 *)&mbox->m_out.xferaddr, (u32 *)&seg);
>>>+ (dma_addr_t *)&mbox->m_out.xferaddr, (u32 *)&seg);
>>
>>Casting just hides a bug.
>
>
> Well, the xferaddr is actually a dma_addr_t now, so that cast really
> does nothing, the only reason it's there is because they previously
> casted it as (u32 *). I removed the cast just so it couldn't obscure
> warnings, and it didn't.
>
> Also, I use dma_addr_t even though it may have nothing to do with dma
> where it's used. I'm more familiar with userland stuff, so I wasn't sure
> what to use. In userland I'd use uintptr_t.
>
>
>>The real fix is to pass a full 64-bit address
>
>
> I did find a few instances where they recast the addresses,
> which was improper, but it does appear that the original address
> in the original driver was coming in as 64bit (dma_addr_t as originally
> written), but were passing it around and casting it as a u32, so I
> think the interfaces allowed for it to work, they just wrote it
> unware of 64bit systems.
>
> Also, they tried to stuff the address returned here :
> ext_inq = pci_alloc_consistent(adapter->dev,
> sizeof(mraid_ext_inquiry), &dma_handle);
> (the dma_handle) into a u32 which I just fixed.
>
>
>>into the s/g list, if it supports 64-bit addresses. if it doesn't, you
>>need to make sure the driver doesn't set highmem_io, make sure the
>>driver doesn't set a 64-bit DMA mask, and make sure the driver does set
>>a 32-bit DMA mask.
>
>
> The driver already does this it appears, without me needing to do it,
> Part of which is covered by this:
> /* Set the Mode of addressing to 64 bit if we can */
> if((adapter->flag & BOARD_64BIT)&&(sizeof(dma_addr_t) ==
> 8)) {
> pci_set_dma_mask(pdev, 0xffffffffffffffffULL);
> adapter->has_64bit_addr = 1;
> }
> else {
> pci_set_dma_mask(pdev, 0xffffffff);
> adapter->has_64bit_addr = 0;
> }

This code is completely bogus and wrong (not your fault, but still).
Take a look at tg3.c for the right way to do it.

The driver continues to have obvious 64-bit issues that your patch
doesn't address. Your main test platform should really be a 32-bit
system with PAE, and >4GB of RAM.

Jeff



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:59    [W:0.038 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site