Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v2)

2007-11-21 Thread Vincent Fortier
Le mardi 20 novembre 2007 à 18:56 -0600, Robert Hancock a écrit :
> This fixes some problems with ATAPI devices on nForce4 controllers in ADMA
> mode on systems with memory located above 4GB. We need to delay setting the
> 64-bit DMA mask until the PRD table and padding buffer are allocated so that
> they don't get allocated above 4GB and break legacy mode (which is needed for
> ATAPI devices). Also, explicitly set a 32-bit DMA mask before allocating the
> legacy buffers since setting the DMA mask affects both ports and we need to
> ensure the second port's buffers are allocated properly (fixes a problem
> with the previous version of this patch).
> 
> Signed-off-by: Robert Hancock <[EMAIL PROTECTED]>
> 

Would this be worth sending to stable team for 2.6.22 & 2.6.23 ?

Regards,

- vin
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v2)

2007-11-21 Thread Robert Hancock

Vincent Fortier wrote:

Le mardi 20 novembre 2007 à 18:56 -0600, Robert Hancock a écrit :

This fixes some problems with ATAPI devices on nForce4 controllers in ADMA
mode on systems with memory located above 4GB. We need to delay setting the
64-bit DMA mask until the PRD table and padding buffer are allocated so that
they don't get allocated above 4GB and break legacy mode (which is needed for
ATAPI devices). Also, explicitly set a 32-bit DMA mask before allocating the
legacy buffers since setting the DMA mask affects both ports and we need to
ensure the second port's buffers are allocated properly (fixes a problem
with the previous version of this patch).

Signed-off-by: Robert Hancock <[EMAIL PROTECTED]>



Would this be worth sending to stable team for 2.6.22 & 2.6.23 ?


Likely (after it gets merged), those versions would have the same bug..
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v2)

2007-11-21 Thread Tejun Heo
Hello, Robert.

Robert Hancock wrote:
> This fixes some problems with ATAPI devices on nForce4 controllers in ADMA
> mode on systems with memory located above 4GB. We need to delay setting the
> 64-bit DMA mask until the PRD table and padding buffer are allocated so that
> they don't get allocated above 4GB and break legacy mode (which is needed for
> ATAPI devices). Also, explicitly set a 32-bit DMA mask before allocating the
> legacy buffers since setting the DMA mask affects both ports and we need to
> ensure the second port's buffers are allocated properly (fixes a problem
> with the previous version of this patch).
> 
> Signed-off-by: Robert Hancock <[EMAIL PROTECTED]>
> 
> + /* Ensure DMA mask is set to 32-bit before allocating legacy PRD and
> +pad buffers */
> + pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
> + pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
[--snip--]
> + pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
> + pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));

I'm probably being paranoid here but please add error checks.  Just
checking return value and returning error suffices.

Thanks.

-- 
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v2)

2007-11-22 Thread Robert Hancock

Tejun Heo wrote:

Hello, Robert.

Robert Hancock wrote:

This fixes some problems with ATAPI devices on nForce4 controllers in ADMA
mode on systems with memory located above 4GB. We need to delay setting the
64-bit DMA mask until the PRD table and padding buffer are allocated so that
they don't get allocated above 4GB and break legacy mode (which is needed for
ATAPI devices). Also, explicitly set a 32-bit DMA mask before allocating the
legacy buffers since setting the DMA mask affects both ports and we need to
ensure the second port's buffers are allocated properly (fixes a problem
with the previous version of this patch).

Signed-off-by: Robert Hancock <[EMAIL PROTECTED]>

+   /* Ensure DMA mask is set to 32-bit before allocating legacy PRD and
+  pad buffers */
+   pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
+   pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));

[--snip--]

+   pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
+   pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));


I'm probably being paranoid here but please add error checks.  Just
checking return value and returning error suffices.


In the 32-bit case, I'm pretty sure those are guaranteed not to fail 
because 32-bit is the default. For the 64-bit ones, we don't care if 
they fail, because then we'll just use whatever mask ends up being set 
(we store the actual set DMA mask in adma_dma_mask for use when we need 
to reconfigure the bounce limit). We definitely don't want to fail 
initialization if the 64-bit set doesn't succeed..

-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v2)

2007-11-22 Thread Tejun Heo
Robert Hancock wrote:
>>> +/* Ensure DMA mask is set to 32-bit before allocating legacy PRD
>>> and
>>> +   pad buffers */
>>> +pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
>>> +pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
>> [--snip--]
>>> +pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
>>> +pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
>>
>> I'm probably being paranoid here but please add error checks.  Just
>> checking return value and returning error suffices.
> 
> In the 32-bit case, I'm pretty sure those are guaranteed not to fail
> because 32-bit is the default. For the 64-bit ones, we don't care if
> they fail, because then we'll just use whatever mask ends up being set
> (we store the actual set DMA mask in adma_dma_mask for use when we need
> to reconfigure the bounce limit). We definitely don't want to fail
> initialization if the 64-bit set doesn't succeed..

Then please add BUG or WARN_ON after 32bit switching (but then again if
you're gonna do that why not just add if (rc) return rc?) and add
comment stating setting 64 bit dma masks is allowed to fail.

Thanks.

-- 
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html