* [PATCH] aic79xx: use dma_get_required_mask()
@ 2007-02-07 8:47 Hannes Reinecke
2007-02-08 0:27 ` James Bottomley
2007-02-08 19:40 ` Christoph Hellwig
0 siblings, 2 replies; 6+ messages in thread
From: Hannes Reinecke @ 2007-02-07 8:47 UTC (permalink / raw)
To: James Bottomley; +Cc: SCSI Mailing List, Frederic TEMPORELLI
[-- Attachment #1: Type: text/plain, Size: 425 bytes --]
Hi James,
this patch corrects the issue originally noted by Frederic Temporelli:
aic79xx uses the wrong logic to determine the addressing mode.
I've converted the original patch to use the dma_get_required_mask()
macros as you requested.
Please apply.
Cheers,
Hannes
--
Dr. Hannes Reinecke hare@suse.de
SuSE Linux Products GmbH S390 & zSeries
Maxfeldstraße 5 +49 911 74053 688
90409 Nürnberg http://www.suse.de
[-- Attachment #2: aic79xx-use-dma-required-mask --]
[-- Type: text/plain, Size: 3058 bytes --]
aic79xx: Use dma_get_required_mask()
As originally noted by Frederic Temporelli, the aic79xx supports 64
bit addressing, but the initialization code of the driver is wrong: it
tests the available memory size instead of testing the maximum
available memory address.
This patch uses the correct dma_get_required_mask() macros to
determine the correct addressing method.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Cc: Xavier Bru <xavier.bru@bull.net>
CC: Frederic Temporelli <frederic.temporelli@bull.net>
diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c b/drivers/scsi/aic7xxx/aic79xx_osm.c
index 9bfcca5..c7fe478 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
@@ -1126,15 +1126,6 @@ ahd_linux_register_host(struct ahd_softc
return 0;
}
-uint64_t
-ahd_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/aic79xx_osm.h b/drivers/scsi/aic7xxx/aic79xx_osm.h
index 3a67fc5..147c83c 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.h
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.h
@@ -496,8 +496,6 @@ ahd_insb(struct ahd_softc * ahd, long po
int ahd_linux_register_host(struct ahd_softc *,
struct scsi_host_template *);
-uint64_t ahd_linux_get_memsize(void);
-
/*************************** Pretty Printing **********************************/
struct info_str {
char *buffer;
diff --git a/drivers/scsi/aic7xxx/aic79xx_osm_pci.c b/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
index 1a3ab6a..22250e6 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
@@ -127,11 +127,13 @@ static int
ahd_linux_pci_dev_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
char buf[80];
+ const uint64_t mask_39bit = 0x7FFFFFFFFFULL;
struct ahd_softc *ahd;
ahd_dev_softc_t pci;
struct ahd_pci_identity *entry;
char *name;
int error;
+ struct device *dev = &pdev->dev;
pci = pdev;
entry = ahd_find_pci_device(pci);
@@ -161,20 +163,17 @@ ahd_linux_pci_dev_probe(struct pci_dev *
pci_set_master(pdev);
if (sizeof(dma_addr_t) > 4) {
- uint64_t memsize;
- const uint64_t mask_39bit = 0x7FFFFFFFFFULL;
-
- memsize = ahd_linux_get_memsize();
-
- if (memsize >= 0x8000000000ULL
- && pci_set_dma_mask(pdev, DMA_64BIT_MASK) == 0) {
+ if (dma_set_mask(dev, DMA_64BIT_MASK)
+ && dma_get_required_mask(dev) > mask_39bit)
ahd->flags |= AHD_64BIT_ADDRESSING;
- } else if (memsize > 0x80000000
- && pci_set_dma_mask(pdev, mask_39bit) == 0) {
+ else if (dma_set_mask(dev, mask_39bit) == 0
+ && dma_get_required_mask(dev) > DMA_32BIT_MASK)
ahd->flags |= AHD_39BIT_ADDRESSING;
- }
+ else
+ dma_set_mask(dev, DMA_32BIT_MASK);
+
} else {
- pci_set_dma_mask(pdev, DMA_32BIT_MASK);
+ dma_set_mask(dev, DMA_32BIT_MASK);
}
ahd->dev_softc = pci;
error = ahd_pci_config(ahd, entry);
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] aic79xx: use dma_get_required_mask()
2007-02-07 8:47 [PATCH] aic79xx: use dma_get_required_mask() Hannes Reinecke
@ 2007-02-08 0:27 ` James Bottomley
2007-02-08 19:40 ` Christoph Hellwig
1 sibling, 0 replies; 6+ messages in thread
From: James Bottomley @ 2007-02-08 0:27 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: SCSI Mailing List, Frederic TEMPORELLI
On Wed, 2007-02-07 at 09:47 +0100, Hannes Reinecke wrote:
> + const uint64_t mask_39bit = 0x7FFFFFFFFFULL;
Actually there's no need for this ... the mask_39bit references can just
become DMA_39BIT_MASK constants. I made this cosmetic change to the
patch.
James
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aic79xx: use dma_get_required_mask()
2007-02-07 8:47 [PATCH] aic79xx: use dma_get_required_mask() Hannes Reinecke
2007-02-08 0:27 ` James Bottomley
@ 2007-02-08 19:40 ` Christoph Hellwig
2007-02-09 15:55 ` James Bottomley
1 sibling, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2007-02-08 19:40 UTC (permalink / raw)
To: Hannes Reinecke; +Cc: James Bottomley, SCSI Mailing List, Frederic TEMPORELLI
> ahd_linux_pci_dev_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> {
> char buf[80];
> + const uint64_t mask_39bit = 0x7FFFFFFFFFULL;
> struct ahd_softc *ahd;
> ahd_dev_softc_t pci;
> struct ahd_pci_identity *entry;
> char *name;
> int error;
> + struct device *dev = &pdev->dev;
>
> pci = pdev;
> entry = ahd_find_pci_device(pci);
> @@ -161,20 +163,17 @@ ahd_linux_pci_dev_probe(struct pci_dev *
> pci_set_master(pdev);
>
> if (sizeof(dma_addr_t) > 4) {
> - uint64_t memsize;
> - const uint64_t mask_39bit = 0x7FFFFFFFFFULL;
> -
> - memsize = ahd_linux_get_memsize();
> -
> - if (memsize >= 0x8000000000ULL
> - && pci_set_dma_mask(pdev, DMA_64BIT_MASK) == 0) {
> + if (dma_set_mask(dev, DMA_64BIT_MASK)
> + && dma_get_required_mask(dev) > mask_39bit)
> ahd->flags |= AHD_64BIT_ADDRESSING;
> - } else if (memsize > 0x80000000
> - && pci_set_dma_mask(pdev, mask_39bit) == 0) {
> + else if (dma_set_mask(dev, mask_39bit) == 0
> + && dma_get_required_mask(dev) > DMA_32BIT_MASK)
> ahd->flags |= AHD_39BIT_ADDRESSING;
> - }
> + else
> + dma_set_mask(dev, DMA_32BIT_MASK);
> +
> } else {
> - pci_set_dma_mask(pdev, DMA_32BIT_MASK);
> + dma_set_mask(dev, DMA_32BIT_MASK);
The logic here seems odd to me. Shouldn't we first check
dma_get_required_mask and the do dma_set_mask?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aic79xx: use dma_get_required_mask()
2007-02-08 19:40 ` Christoph Hellwig
@ 2007-02-09 15:55 ` James Bottomley
2007-02-09 16:10 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: James Bottomley @ 2007-02-09 15:55 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Hannes Reinecke, SCSI Mailing List, Frederic TEMPORELLI
On Thu, 2007-02-08 at 19:40 +0000, Christoph Hellwig wrote:
> The logic here seems odd to me. Shouldn't we first check
> dma_get_required_mask and the do dma_set_mask?
Yes ... I picked up the mask problem, but not that. And actually
there's a missing zero check on one of the dma_set_mask()s.
Does this look like the right patch then?
James
>From 095ec6c45d80171b31ee04da62618dcad31b8aef Mon Sep 17 00:00:00 2001
From: Hannes Reinecke <hare@suse.de>
Date: Wed, 7 Feb 2007 09:47:44 +0100
Subject: [SCSI] aic79xx: use dma_get_required_mask()
As originally noted by Frederic Temporelli, the aic79xx supports 64
bit addressing, but the initialization code of the driver is wrong: it
tests the available memory size instead of testing the maximum
available memory address.
This patch uses the correct dma_get_required_mask() macros to
determine the correct addressing method.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Cc: Xavier Bru <xavier.bru@bull.net>
CC: Frederic Temporelli <frederic.temporelli@bull.net>
cosmetic fixes
Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
---
drivers/scsi/aic7xxx/aic79xx_osm.c | 9 ---------
drivers/scsi/aic7xxx/aic79xx_osm.h | 2 --
drivers/scsi/aic7xxx/aic79xx_osm_pci.c | 20 +++++++++-----------
3 files changed, 9 insertions(+), 22 deletions(-)
Index: scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm.c
===================================================================
--- scsi-misc-2.6.orig/drivers/scsi/aic7xxx/aic79xx_osm.c 2007-02-09 09:12:55.000000000 -0500
+++ scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm.c 2007-02-09 09:17:36.000000000 -0500
@@ -1126,15 +1126,6 @@ ahd_linux_register_host(struct ahd_softc
return 0;
}
-uint64_t
-ahd_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
Index: scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm.h
===================================================================
--- scsi-misc-2.6.orig/drivers/scsi/aic7xxx/aic79xx_osm.h 2007-02-09 09:12:55.000000000 -0500
+++ scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm.h 2007-02-09 09:17:36.000000000 -0500
@@ -496,8 +496,6 @@ ahd_insb(struct ahd_softc * ahd, long po
int ahd_linux_register_host(struct ahd_softc *,
struct scsi_host_template *);
-uint64_t ahd_linux_get_memsize(void);
-
/*************************** Pretty Printing **********************************/
struct info_str {
char *buffer;
Index: scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
===================================================================
--- scsi-misc-2.6.orig/drivers/scsi/aic7xxx/aic79xx_osm_pci.c 2007-02-09 09:12:55.000000000 -0500
+++ scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm_pci.c 2007-02-09 09:21:32.000000000 -0500
@@ -132,6 +132,7 @@ ahd_linux_pci_dev_probe(struct pci_dev *
struct ahd_pci_identity *entry;
char *name;
int error;
+ struct device *dev = &pdev->dev;
pci = pdev;
entry = ahd_find_pci_device(pci);
@@ -161,20 +162,16 @@ ahd_linux_pci_dev_probe(struct pci_dev *
pci_set_master(pdev);
if (sizeof(dma_addr_t) > 4) {
- uint64_t memsize;
- const uint64_t mask_39bit = 0x7FFFFFFFFFULL;
-
- memsize = ahd_linux_get_memsize();
-
- if (memsize >= 0x8000000000ULL
- && pci_set_dma_mask(pdev, DMA_64BIT_MASK) == 0) {
+ if (dma_get_required_mask(dev) > DMA_39BIT_MASK
+ && dma_set_mask(dev, DMA_64BIT_MASK) == 0)
ahd->flags |= AHD_64BIT_ADDRESSING;
- } else if (memsize > 0x80000000
- && pci_set_dma_mask(pdev, mask_39bit) == 0) {
+ else if (dma_get_required_mask(dev) > DMA_32BIT_MASK
+ && dma_set_mask(dev, DMA_39BIT_MASK) == 0)
ahd->flags |= AHD_39BIT_ADDRESSING;
- }
+ else
+ dma_set_mask(dev, DMA_32BIT_MASK);
} else {
- pci_set_dma_mask(pdev, DMA_32BIT_MASK);
+ dma_set_mask(dev, DMA_32BIT_MASK);
}
ahd->dev_softc = pci;
error = ahd_pci_config(ahd, entry);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aic79xx: use dma_get_required_mask()
2007-02-09 15:55 ` James Bottomley
@ 2007-02-09 16:10 ` Christoph Hellwig
2007-02-09 16:36 ` James Bottomley
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2007-02-09 16:10 UTC (permalink / raw)
To: James Bottomley
Cc: Christoph Hellwig, Hannes Reinecke, SCSI Mailing List,
Frederic TEMPORELLI
On Fri, Feb 09, 2007 at 10:55:39AM -0500, James Bottomley wrote:
> On Thu, 2007-02-08 at 19:40 +0000, Christoph Hellwig wrote:
> > The logic here seems odd to me. Shouldn't we first check
> > dma_get_required_mask and the do dma_set_mask?
>
> Yes ... I picked up the mask problem, but not that. And actually
> there's a missing zero check on one of the dma_set_mask()s.
>
> Does this look like the right patch then?
This looks functionally correct to me, but there's some cosmetic issues
left:
> Index: scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
> ===================================================================
> --- scsi-misc-2.6.orig/drivers/scsi/aic7xxx/aic79xx_osm_pci.c 2007-02-09 09:12:55.000000000 -0500
> +++ scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm_pci.c 2007-02-09 09:21:32.000000000 -0500
> @@ -132,6 +132,7 @@ ahd_linux_pci_dev_probe(struct pci_dev *
> struct ahd_pci_identity *entry;
> char *name;
> int error;
> + struct device *dev = &pdev->dev;
>
> pci = pdev;
> entry = ahd_find_pci_device(pci);
> @@ -161,20 +162,16 @@ ahd_linux_pci_dev_probe(struct pci_dev *
> pci_set_master(pdev);
>
> if (sizeof(dma_addr_t) > 4) {
> - uint64_t memsize;
> - const uint64_t mask_39bit = 0x7FFFFFFFFFULL;
> -
> - memsize = ahd_linux_get_memsize();
> -
> - if (memsize >= 0x8000000000ULL
> - && pci_set_dma_mask(pdev, DMA_64BIT_MASK) == 0) {
> + if (dma_get_required_mask(dev) > DMA_39BIT_MASK
> + && dma_set_mask(dev, DMA_64BIT_MASK) == 0)
> ahd->flags |= AHD_64BIT_ADDRESSING;
> - } else if (memsize > 0x80000000
> - && pci_set_dma_mask(pdev, mask_39bit) == 0) {
> + else if (dma_get_required_mask(dev) > DMA_32BIT_MASK
> + && dma_set_mask(dev, DMA_39BIT_MASK) == 0)
> ahd->flags |= AHD_39BIT_ADDRESSING;
> - }
> + else
> + dma_set_mask(dev, DMA_32BIT_MASK);
> } else {
I'd rather do the dma_get_required_mask only once, and we we want
&& at the end of the first instead of at the beginning of the second
line. So this block should look like:
u64 required_mask = dma_get_required_mask(dev);
if (required_mask > DMA_39BIT_MASK &&
dma_set_mask(dev, DMA_64BIT_MASK) == 0)
ahd->flags |= AHD_64BIT_ADDRESSING;
else if (required_mask > DMA_32BIT_MASK &&
dma_set_mask(dev, DMA_39BIT_MASK) == 0)
ahd->flags |= AHD_39BIT_ADDRESSING;
else
dma_set_mask(dev, DMA_32BIT_MASK);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aic79xx: use dma_get_required_mask()
2007-02-09 16:10 ` Christoph Hellwig
@ 2007-02-09 16:36 ` James Bottomley
0 siblings, 0 replies; 6+ messages in thread
From: James Bottomley @ 2007-02-09 16:36 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Hannes Reinecke, SCSI Mailing List, Frederic TEMPORELLI
On Fri, 2007-02-09 at 16:10 +0000, Christoph Hellwig wrote:
> On Fri, Feb 09, 2007 at 10:55:39AM -0500, James Bottomley wrote:
> > On Thu, 2007-02-08 at 19:40 +0000, Christoph Hellwig wrote:
> > > The logic here seems odd to me. Shouldn't we first check
> > > dma_get_required_mask and the do dma_set_mask?
> >
> > Yes ... I picked up the mask problem, but not that. And actually
> > there's a missing zero check on one of the dma_set_mask()s.
> >
> > Does this look like the right patch then?
>
> This looks functionally correct to me, but there's some cosmetic issues
> left:
OK, try this one.
By the way, when did I suddenly become your patch monkey?
James
>From 095ec6c45d80171b31ee04da62618dcad31b8aef Mon Sep 17 00:00:00 2001
From: Hannes Reinecke <hare@suse.de>
Date: Wed, 7 Feb 2007 09:47:44 +0100
Subject: [SCSI] aic79xx: use dma_get_required_mask()
As originally noted by Frederic Temporelli, the aic79xx supports 64
bit addressing, but the initialization code of the driver is wrong: it
tests the available memory size instead of testing the maximum
available memory address.
This patch uses the correct dma_get_required_mask() macros to
determine the correct addressing method.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Cc: Xavier Bru <xavier.bru@bull.net>
CC: Frederic Temporelli <frederic.temporelli@bull.net>
cosmetic fixes
Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
---
drivers/scsi/aic7xxx/aic79xx_osm.c | 9 ---------
drivers/scsi/aic7xxx/aic79xx_osm.h | 2 --
drivers/scsi/aic7xxx/aic79xx_osm_pci.c | 20 +++++++++-----------
3 files changed, 9 insertions(+), 22 deletions(-)
Index: scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm.c
===================================================================
--- scsi-misc-2.6.orig/drivers/scsi/aic7xxx/aic79xx_osm.c 2007-02-09 09:12:55.000000000 -0500
+++ scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm.c 2007-02-09 09:17:36.000000000 -0500
@@ -1126,15 +1126,6 @@ ahd_linux_register_host(struct ahd_softc
return 0;
}
-uint64_t
-ahd_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
Index: scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm.h
===================================================================
--- scsi-misc-2.6.orig/drivers/scsi/aic7xxx/aic79xx_osm.h 2007-02-09 09:12:55.000000000 -0500
+++ scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm.h 2007-02-09 09:17:36.000000000 -0500
@@ -496,8 +496,6 @@ ahd_insb(struct ahd_softc * ahd, long po
int ahd_linux_register_host(struct ahd_softc *,
struct scsi_host_template *);
-uint64_t ahd_linux_get_memsize(void);
-
/*************************** Pretty Printing **********************************/
struct info_str {
char *buffer;
Index: scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
===================================================================
--- scsi-misc-2.6.orig/drivers/scsi/aic7xxx/aic79xx_osm_pci.c 2007-02-09 09:12:55.000000000 -0500
+++ scsi-misc-2.6/drivers/scsi/aic7xxx/aic79xx_osm_pci.c 2007-02-09 11:21:41.000000000 -0500
@@ -132,6 +132,7 @@ ahd_linux_pci_dev_probe(struct pci_dev *
struct ahd_pci_identity *entry;
char *name;
int error;
+ struct device *dev = &pdev->dev;
pci = pdev;
entry = ahd_find_pci_device(pci);
@@ -161,20 +162,18 @@ ahd_linux_pci_dev_probe(struct pci_dev *
pci_set_master(pdev);
if (sizeof(dma_addr_t) > 4) {
- uint64_t memsize;
- const uint64_t mask_39bit = 0x7FFFFFFFFFULL;
+ const u64 required_mask = dma_get_required_mask(dev);
- memsize = ahd_linux_get_memsize();
-
- if (memsize >= 0x8000000000ULL
- && pci_set_dma_mask(pdev, DMA_64BIT_MASK) == 0) {
+ if (required_mask > DMA_39BIT_MASK &&
+ dma_set_mask(dev, DMA_64BIT_MASK) == 0)
ahd->flags |= AHD_64BIT_ADDRESSING;
- } else if (memsize > 0x80000000
- && pci_set_dma_mask(pdev, mask_39bit) == 0) {
+ else if (required_mask > DMA_32BIT_MASK &&
+ dma_set_mask(dev, DMA_39BIT_MASK) == 0)
ahd->flags |= AHD_39BIT_ADDRESSING;
- }
+ else
+ dma_set_mask(dev, DMA_32BIT_MASK);
} else {
- pci_set_dma_mask(pdev, DMA_32BIT_MASK);
+ dma_set_mask(dev, DMA_32BIT_MASK);
}
ahd->dev_softc = pci;
error = ahd_pci_config(ahd, entry);
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-02-09 16:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-07 8:47 [PATCH] aic79xx: use dma_get_required_mask() Hannes Reinecke
2007-02-08 0:27 ` James Bottomley
2007-02-08 19:40 ` Christoph Hellwig
2007-02-09 15:55 ` James Bottomley
2007-02-09 16:10 ` Christoph Hellwig
2007-02-09 16:36 ` James Bottomley
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.