[PATCH] aic7xxx: move to dma_get_required_mask() and correct 39 bit assumptions

[PATCH] aic7xxx: move to dma_get_required_mask() and correct 39 bit assumptions

[James Bottomley]

This patch moves aic7xxx over to the dma_get_required_mask() API and
dumps its open coded memory check.

It also appears from this bug:

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=167049

That 39 bit addressing doesn't work on older cards.  I surmise that the
AHC_LARGE_SCBS flag is the one that marks cards capable of using 39 bit
addressing, so I also folded that check into the code.

Luben, since you have access to the docs, could you confirm this guess.

Dave, could you see if this fixes the problem for the bug reporter.

James

diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.c b/drivers/scsi/aic7xxx/aic7xxx_osm.c
--- a/drivers/scsi/aic7xxx/aic7xxx_osm.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm.c
@@ -1036,15 +1036,6 @@ ahc_linux_register_host(struct ahc_softc
 	return (0);
 }
 
-uint64_t
-ahc_linux_get_memsize(void)
-{
-	struct sysinfo si;
-
-	si_meminfo(&si);
-	return ((uint64_t)si.totalram << PAGE_SHIFT);
-}
-
 /*
  * Place the SCSI bus into a known state by either resetting it,
  * or forcing transfer negotiations on the next command to any
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.h b/drivers/scsi/aic7xxx/aic7xxx_osm.h
--- a/drivers/scsi/aic7xxx/aic7xxx_osm.h
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm.h
@@ -494,8 +494,6 @@ ahc_insb(struct ahc_softc * ahc, long po
 int		ahc_linux_register_host(struct ahc_softc *,
 					struct scsi_host_template *);
 
-uint64_t	ahc_linux_get_memsize(void);
-
 /*************************** Pretty Printing **********************************/
 struct info_str {
 	char *buffer;
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c b/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
--- a/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
@@ -180,6 +180,7 @@ ahc_linux_pci_dev_probe(struct pci_dev *
 	struct		 ahc_pci_identity *entry;
 	char		*name;
 	int		 error;
+	struct device	*dev = &pdev->dev;
 
 	pci = pdev;
 	entry = ahc_find_pci_device(pci);
@@ -209,11 +210,12 @@ ahc_linux_pci_dev_probe(struct pci_dev *
 	pci_set_master(pdev);
 
 	if (sizeof(dma_addr_t) > 4
-	 && ahc_linux_get_memsize() > 0x80000000
-	 && pci_set_dma_mask(pdev, mask_39bit) == 0) {
+	    && ahc->features & AHC_LARGE_SCBS
+	    && dma_set_mask(dev, mask_39bit) == 0
+	    && dma_get_required_mask(dev) > DMA_32BIT_MASK) {
 		ahc->flags |= AHC_39BIT_ADDRESSING;
 	} else {
-		if (pci_set_dma_mask(pdev, DMA_32BIT_MASK)) {
+		if (dma_set_mask(dev, DMA_32BIT_MASK)) {
 			printk(KERN_WARNING "aic7xxx: No suitable DMA available.\n");
                 	return (-ENODEV);
 		}

Re: [PATCH] aic7xxx: move to dma_get_required_mask() and correct 39 bit assumptions

[Christoph Hellwig]

On Tue, Aug 30, 2005 at 11:48:07AM -0500, James Bottomley wrote:
> This patch moves aic7xxx over to the dma_get_required_mask() API and
> dumps its open coded memory check.

Can you put in that change for aic79xx aswell?

Re: [PATCH] aic7xxx: move to dma_get_required_mask() and correct 39 bit assumptions

[Luben Tuikov]

--- James Bottomley <James.Bottomley@SteelEye.com> wrote:

> This patch moves aic7xxx over to the dma_get_required_mask() API and
> dumps its open coded memory check.
> 
> It also appears from this bug:
> 
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=167049
> 
> That 39 bit addressing doesn't work on older cards.  I surmise that the
> AHC_LARGE_SCBS flag is the one that marks cards capable of using 39 bit
> addressing, so I also folded that check into the code.
> 
> Luben, since you have access to the docs, could you confirm this guess.

AHC_LARGE_SCBS means that the card can take 64 byte hardware SCBs.
Normally HSCBs are 32 bytes for older controllers.  Latter aic78xx and
all aic79xx chips support 64 byte HSCBs.  So this isn't related.

To fix this bug, you should only have to substitute
           ahc_linux_get_memsize() > 0x80000000
with
           dma_get_required_mask(dev) > DMA_32BIT_MASK,
to fix this issue (unless of course the kernel is confused too ;-) ).

All that the controller cares is if it will have to use 39 bits
to do DMA.  You want to set the DMA mask accordingly to how much
memory will have to be addressed _and_ where it will have to be
addressed.

You may also want to use Intel's PAE (64 MB) for doing that kind of
memory remap.

      Luben

> 
> Dave, could you see if this fixes the problem for the bug reporter.
> 
> James
> 
> diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.c
> b/drivers/scsi/aic7xxx/aic7xxx_osm.c
> --- a/drivers/scsi/aic7xxx/aic7xxx_osm.c
> +++ b/drivers/scsi/aic7xxx/aic7xxx_osm.c
> @@ -1036,15 +1036,6 @@ ahc_linux_register_host(struct ahc_softc
>  	return (0);
>  }
>  
> -uint64_t
> -ahc_linux_get_memsize(void)
> -{
> -	struct sysinfo si;
> -
> -	si_meminfo(&si);
> -	return ((uint64_t)si.totalram << PAGE_SHIFT);
> -}
> -
>  /*
>   * Place the SCSI bus into a known state by either resetting it,
>   * or forcing transfer negotiations on the next command to any
> diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.h
> b/drivers/scsi/aic7xxx/aic7xxx_osm.h
> --- a/drivers/scsi/aic7xxx/aic7xxx_osm.h
> +++ b/drivers/scsi/aic7xxx/aic7xxx_osm.h
> @@ -494,8 +494,6 @@ ahc_insb(struct ahc_softc * ahc, long po
>  int		ahc_linux_register_host(struct ahc_softc *,
>  					struct scsi_host_template *);
>  
> -uint64_t	ahc_linux_get_memsize(void);
> -
>  /*************************** Pretty Printing
> **********************************/
>  struct info_str {
>  	char *buffer;
> diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
> b/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
> --- a/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
> +++ b/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
> @@ -180,6 +180,7 @@ ahc_linux_pci_dev_probe(struct pci_dev *
>  	struct		 ahc_pci_identity *entry;
>  	char		*name;
>  	int		 error;
> +	struct device	*dev = &pdev->dev;
>  
>  	pci = pdev;
>  	entry = ahc_find_pci_device(pci);
> @@ -209,11 +210,12 @@ ahc_linux_pci_dev_probe(struct pci_dev *
>  	pci_set_master(pdev);
>  
>  	if (sizeof(dma_addr_t) > 4
> -	 && ahc_linux_get_memsize() > 0x80000000
> -	 && pci_set_dma_mask(pdev, mask_39bit) == 0) {
> +	    && ahc->features & AHC_LARGE_SCBS
> +	    && dma_set_mask(dev, mask_39bit) == 0
> +	    && dma_get_required_mask(dev) > DMA_32BIT_MASK) {
>  		ahc->flags |= AHC_39BIT_ADDRESSING;
>  	} else {
> -		if (pci_set_dma_mask(pdev, DMA_32BIT_MASK)) {
> +		if (dma_set_mask(dev, DMA_32BIT_MASK)) {
>  			printk(KERN_WARNING "aic7xxx: No suitable DMA available.\n");
>                  	return (-ENODEV);
>  		}
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] aic7xxx: move to dma_get_required_mask() and correct 39 bit assumptions

[James Bottomley]

On Tue, 2005-08-30 at 15:13 -0700, Luben Tuikov wrote:
> AHC_LARGE_SCBS means that the card can take 64 byte hardware SCBs.
> Normally HSCBs are 32 bytes for older controllers.  Latter aic78xx and
> all aic79xx chips support 64 byte HSCBs.  So this isn't related.

So all aic chips, regardless of age can do 39 bit addressing mode in the
scb?  It also means they must be DAC capable, which I thought was pretty
much a universally missing feature in early cards.  From the bug report,
the card in question is a 7880, which is pretty early, I think: only
ultra, not even U2 or LVD.

> To fix this bug, you should only have to substitute
>            ahc_linux_get_memsize() > 0x80000000
> with
>            dma_get_required_mask(dev) > DMA_32BIT_MASK,
> to fix this issue (unless of course the kernel is confused too ;-) ).

But all that does is correct a thinko in the original code where it
would try to turn on 39 bit addressing mode if the machine has >2GB of
memory.  The practical effect of which is pretty much nil.

The symptoms reported look very much like DMA to the wrong region.

James

Re: [PATCH] aic7xxx: move to dma_get_required_mask() and correct 39 bit assumptions

[James Bottomley]

On Tue, 2005-08-30 at 17:24 -0500, James Bottomley wrote:
> On Tue, 2005-08-30 at 15:13 -0700, Luben Tuikov wrote:
> > AHC_LARGE_SCBS means that the card can take 64 byte hardware SCBs.
> > Normally HSCBs are 32 bytes for older controllers.  Latter aic78xx and
> > all aic79xx chips support 64 byte HSCBs.  So this isn't related.
> 
> So all aic chips, regardless of age can do 39 bit addressing mode in the
> scb?  It also means they must be DAC capable, which I thought was pretty
> much a universally missing feature in early cards.  From the bug report,
> the card in question is a 7880, which is pretty early, I think: only
> ultra, not even U2 or LVD.
> 
> > To fix this bug, you should only have to substitute
> >            ahc_linux_get_memsize() > 0x80000000
> > with
> >            dma_get_required_mask(dev) > DMA_32BIT_MASK,
> > to fix this issue (unless of course the kernel is confused too ;-) ).
> 
> But all that does is correct a thinko in the original code where it
> would try to turn on 39 bit addressing mode if the machine has >2GB of
> memory.  The practical effect of which is pretty much nil.
> 
> The symptoms reported look very much like DMA to the wrong region.

OK, I just had confirmation that the patch does indeed fix the bug on
x86_64 validating the guess that the 7880 isn't working correctly with
39 bit addresses.

Luben, could you look in the manual and see if it says what chip
versions do work with the 39 bit mask.  For the time being I'm going to
commit the fix that assumes only the versions with AHC_LARGE_SCB's
defined work.  That's probably going to penalise a few chipsets that
have 39 bit but not 64 bit, but without the information, it's the best I
can do.

James