[PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources

[PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources

[pbathija]

From: Pravin Bathija <pbathija@amcc.com>

Powerpc 44x uses 36 bit real address while the real address defined
in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
fails to initialize. This fix changes the data types representing the real
address from unsigned long 32-bit types to "phys_addr_t" which is 64-bit. The
driver has been tested, the disks get discovered correctly and can do IO.

Signed-off-by: Pravin Bathija <pbathija@amcc.com>
Acked-by: Feng Kan <fkan@amcc.com>
Acked-by: Prodyut Hazarika <phazarika@amcc.com>
Acked-by: Loc Ho <lho@amcc.com>
Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
Acked-by: Victor Gallardo <vgallardo@amcc.com>
---
 drivers/message/fusion/mptbase.c |   34 +++++++++++++++++++++++++---------
 drivers/message/fusion/mptbase.h |    5 +++--
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 5d496a9..d5b0f15 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1510,11 +1510,12 @@ static int
 mpt_mapresources(MPT_ADAPTER *ioc)
 {
 	u8		__iomem *mem;
+	u8		__iomem *port;
 	int		 ii;
-	unsigned long	 mem_phys;
-	unsigned long	 port;
-	u32		 msize;
-	u32		 psize;
+	phys_addr_t	 mem_phys;
+	phys_addr_t	 port_phys;
+	resource_size_t	 msize;
+	resource_size_t	 psize;
 	u8		 revision;
 	int		 r = -ENODEV;
 	struct pci_dev *pdev;
@@ -1552,13 +1553,13 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 	}
 
 	mem_phys = msize = 0;
-	port = psize = 0;
+	port_phys = psize = 0;
 	for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) {
 		if (pci_resource_flags(pdev, ii) & PCI_BASE_ADDRESS_SPACE_IO) {
 			if (psize)
 				continue;
 			/* Get I/O space! */
-			port = pci_resource_start(pdev, ii);
+			port_phys = pci_resource_start(pdev, ii);
 			psize = pci_resource_len(pdev, ii);
 		} else {
 			if (msize)
@@ -1580,14 +1581,23 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 		return -EINVAL;
 	}
 	ioc->memmap = mem;
-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
-	    ioc->name, mem, mem_phys));
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
+	    ioc->name, mem, (u64)mem_phys));
 
 	ioc->mem_phys = mem_phys;
 	ioc->chip = (SYSIF_REGS __iomem *)mem;
 
 	/* Save Port IO values in case we need to do downloadboot */
-	ioc->pio_mem_phys = port;
+	port = ioremap(port_phys, psize);
+	if (port == NULL) {
+		printk(MYIOC_s_ERR_FMT " : ERROR - Unable to map adapter"
+			" port !\n", ioc->name);
+		return -EINVAL;
+	}
+	ioc->portmap = port;
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "port=%p, port_phys=%llx\n",
+			ioc->name, port, (u64)port_phys));
+	ioc->pio_mem_phys = port_phys;
 	ioc->pio_chip = (SYSIF_REGS __iomem *)port;
 
 	return 0;
@@ -1822,6 +1832,7 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 		if (ioc->alt_ioc)
 			ioc->alt_ioc->alt_ioc = NULL;
 		iounmap(ioc->memmap);
+		iounmap(ioc->portmap);
 		if (r != -5)
 			pci_release_selected_regions(pdev, ioc->bars);
 
@@ -2583,6 +2594,11 @@ mpt_adapter_dispose(MPT_ADAPTER *ioc)
 		ioc->memmap = NULL;
 	}
 
+	if (ioc->portmap != NULL) {
+		iounmap(ioc->portmap);
+		ioc->portmap = NULL;
+	}
+
 	pci_disable_device(ioc->pcidev);
 	pci_release_selected_regions(ioc->pcidev, ioc->bars);
 
diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index b3e981d..8e12bf8 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -584,8 +584,8 @@ typedef struct _MPT_ADAPTER
 	SYSIF_REGS __iomem	*chip;		/* == c8817000 (mmap) */
 	SYSIF_REGS __iomem	*pio_chip;	/* Programmed IO (downloadboot) */
 	u8			 bus_type;
-	u32			 mem_phys;	/* == f4020000 (mmap) */
-	u32			 pio_mem_phys;	/* Programmed IO (downloadboot) */
+	phys_addr_t		 mem_phys;	/* == f4020000 (mmap) */
+	phys_addr_t		 pio_mem_phys;	/* Programmed IO (downloadboot) */
 	int			 mem_size;	/* mmap memory size */
 	int			 number_of_buses;
 	int			 devices_per_bus;
@@ -635,6 +635,7 @@ typedef struct _MPT_ADAPTER
 	int			bars;		/* bitmask of BAR's that must be configured */
 	int			msi_enable;
 	u8			__iomem *memmap;	/* mmap address */
+	u8			__iomem *portmap;	/* mmap port address */
 	struct Scsi_Host	*sh;		/* Scsi Host pointer */
 	SpiCfgData		spi_data;	/* Scsi config. data */
 	RaidCfgData		raid_data;	/* Raid config. data */
-- 
1.5.5

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources

[Benjamin Herrenschmidt]

> diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> index 5d496a9..d5b0f15 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -1510,11 +1510,12 @@ static int
>  mpt_mapresources(MPT_ADAPTER *ioc)
>  {
>  	u8		__iomem *mem;
> +	u8		__iomem *port;
>  	int		 ii;
> -	unsigned long	 mem_phys;
> -	unsigned long	 port;
> -	u32		 msize;
> -	u32		 psize;
> +	phys_addr_t	 mem_phys;
> +	phys_addr_t	 port_phys;
> +	resource_size_t	 msize;
> +	resource_size_t	 psize;

Is phys_addr_t defined for all archs nowadays ? Why not use
resource_size_t for everything ? resource_size_t is a bit of a misnomer,
it's not a type supposed to reference a "size" but really a physical
address (or a size)... it's been called resource_size_t I believe
because it's "sized" appropriately for holding a physical address.

Cheers,
Ben.

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources

[Benjamin Herrenschmidt]

> diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> index 5d496a9..d5b0f15 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -1510,11 +1510,12 @@ static int
>  mpt_mapresources(MPT_ADAPTER *ioc)
>  {
>  	u8		__iomem *mem;
> +	u8		__iomem *port;
>  	int		 ii;
> -	unsigned long	 mem_phys;
> -	unsigned long	 port;
> -	u32		 msize;
> -	u32		 psize;
> +	phys_addr_t	 mem_phys;
> +	phys_addr_t	 port_phys;
> +	resource_size_t	 msize;
> +	resource_size_t	 psize;

Is phys_addr_t defined for all archs nowadays ? Why not use
resource_size_t for everything ? resource_size_t is a bit of a misnomer,
it's not a type supposed to reference a "size" but really a physical
address (or a size)... it's been called resource_size_t I believe
because it's "sized" appropriately for holding a physical address.

Cheers,
Ben.

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Anatolij Gustschin]

On Thu, 10 Dec 2009 09:43:38 -0600
James Bottomley <James.Bottomley@suse.de> wrote:

> On Wed, 2009-12-02 at 17:51 -0800, Pravin Bathija wrote:
> > Powerpc 44x uses 36 bit real address while the real address defined
> >     in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
> >     fails to initialize. This fix changes the data types representing the real
> >     address from unsigned long 32-bit types to resource_size_t which is 64-bit. The
> >     driver has been tested, the disks get discovered correctly and can do IO.
> > 
> > Signed-off-by: Pravin Bathija <pbathija@amcc.com>
> > Acked-by: Feng Kan <fkan@amcc.com>
> > Acked-by: Fushen Chen <fchen@amcc.com>
> > Acked-by: Loc Ho <lho@amcc.com>
> > Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
> > Acked-by: Victor Gallardo <vgallardo@amcc.com>
> > ---
> >  drivers/message/fusion/mptbase.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> > index 5d496a9..9f14a60 100644
> > --- a/drivers/message/fusion/mptbase.c
> > +++ b/drivers/message/fusion/mptbase.c
> > @@ -1511,7 +1511,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> >  {
> >  	u8		__iomem *mem;
> >  	int		 ii;
> > -	unsigned long	 mem_phys;
> > +	resource_size_t	 mem_phys;
> 
> You never actually compiled this, did you?
> 
> drivers/message/fusion/mptbase.c: In function 'mpt_mapresources':
> drivers/message/fusion/mptbase.c:1680: warning: format '%lx' expects type 'long unsigned int', but argument 4 has type 'resource_size_t'
> 
> I'll just fold the fix in

another patch (inlined below) should probably also go in as 'mem_phys' is
assigned to ioc->mem_phys which is 'u32'. ioc->mem_phys is never used in
the driver, however.

Some time ago I posted a patch which enables using second LSI SAS HBA on
PPC440SPe based katmai board again:

http://thread.gmane.org/gmane.linux.scsi/54839

Could someone comment on this patch, please. Thanks!

Anatolij

---

diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index 8dd4d21..8dc58e3 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -605,7 +605,7 @@ typedef struct _MPT_ADAPTER
 	SYSIF_REGS __iomem	*chip;		/* == c8817000 (mmap) */
 	SYSIF_REGS __iomem	*pio_chip;	/* Programmed IO (downloadboot) */
 	u8			 bus_type;
-	u32			 mem_phys;	/* == f4020000 (mmap) */
+	resource_size_t		 mem_phys;	/* == f4020000 (mmap) */
 	u32			 pio_mem_phys;	/* Programmed IO (downloadboot) */
 	int			 mem_size;	/* mmap memory size */
 	int			 number_of_buses;

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Anatolij Gustschin]

On Thu, 10 Dec 2009 09:43:38 -0600
James Bottomley <James.Bottomley@suse.de> wrote:

> On Wed, 2009-12-02 at 17:51 -0800, Pravin Bathija wrote:
> > Powerpc 44x uses 36 bit real address while the real address defined
> >     in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
> >     fails to initialize. This fix changes the data types representing the real
> >     address from unsigned long 32-bit types to resource_size_t which is 64-bit. The
> >     driver has been tested, the disks get discovered correctly and can do IO.
> > 
> > Signed-off-by: Pravin Bathija <pbathija@amcc.com>
> > Acked-by: Feng Kan <fkan@amcc.com>
> > Acked-by: Fushen Chen <fchen@amcc.com>
> > Acked-by: Loc Ho <lho@amcc.com>
> > Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
> > Acked-by: Victor Gallardo <vgallardo@amcc.com>
> > ---
> >  drivers/message/fusion/mptbase.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> > index 5d496a9..9f14a60 100644
> > --- a/drivers/message/fusion/mptbase.c
> > +++ b/drivers/message/fusion/mptbase.c
> > @@ -1511,7 +1511,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> >  {
> >  	u8		__iomem *mem;
> >  	int		 ii;
> > -	unsigned long	 mem_phys;
> > +	resource_size_t	 mem_phys;
> 
> You never actually compiled this, did you?
> 
> drivers/message/fusion/mptbase.c: In function 'mpt_mapresources':
> drivers/message/fusion/mptbase.c:1680: warning: format '%lx' expects type 'long unsigned int', but argument 4 has type 'resource_size_t'
> 
> I'll just fold the fix in

another patch (inlined below) should probably also go in as 'mem_phys' is
assigned to ioc->mem_phys which is 'u32'. ioc->mem_phys is never used in
the driver, however.

Some time ago I posted a patch which enables using second LSI SAS HBA on
PPC440SPe based katmai board again:

http://thread.gmane.org/gmane.linux.scsi/54839

Could someone comment on this patch, please. Thanks!

Anatolij

---

diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index 8dd4d21..8dc58e3 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -605,7 +605,7 @@ typedef struct _MPT_ADAPTER
 	SYSIF_REGS __iomem	*chip;		/* == c8817000 (mmap) */
 	SYSIF_REGS __iomem	*pio_chip;	/* Programmed IO (downloadboot) */
 	u8			 bus_type;
-	u32			 mem_phys;	/* == f4020000 (mmap) */
+	resource_size_t		 mem_phys;	/* == f4020000 (mmap) */
 	u32			 pio_mem_phys;	/* Programmed IO (downloadboot) */
 	int			 mem_size;	/* mmap memory size */
 	int			 number_of_buses;

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[James Bottomley]

On Wed, 2009-12-02 at 17:51 -0800, Pravin Bathija wrote:
> Powerpc 44x uses 36 bit real address while the real address defined
>     in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
>     fails to initialize. This fix changes the data types representing the real
>     address from unsigned long 32-bit types to resource_size_t which is 64-bit. The
>     driver has been tested, the disks get discovered correctly and can do IO.
> 
> Signed-off-by: Pravin Bathija <pbathija@amcc.com>
> Acked-by: Feng Kan <fkan@amcc.com>
> Acked-by: Fushen Chen <fchen@amcc.com>
> Acked-by: Loc Ho <lho@amcc.com>
> Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
> Acked-by: Victor Gallardo <vgallardo@amcc.com>
> ---
>  drivers/message/fusion/mptbase.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> index 5d496a9..9f14a60 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -1511,7 +1511,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
>  {
>  	u8		__iomem *mem;
>  	int		 ii;
> -	unsigned long	 mem_phys;
> +	resource_size_t	 mem_phys;

You never actually compiled this, did you?

drivers/message/fusion/mptbase.c: In function 'mpt_mapresources':
drivers/message/fusion/mptbase.c:1680: warning: format '%lx' expects type 'long unsigned int', but argument 4 has type 'resource_size_t'

I'll just fold the fix in

James

---

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 162923f..85bc6a6 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1677,8 +1677,8 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 		return -EINVAL;
 	}
 	ioc->memmap = mem;
-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
-	    ioc->name, mem, mem_phys));
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
+	    ioc->name, mem, (unsigned long long)mem_phys));
 
 	ioc->mem_phys = mem_phys;
 	ioc->chip = (SYSIF_REGS __iomem *)mem;

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[James Bottomley]

On Wed, 2009-12-02 at 17:51 -0800, Pravin Bathija wrote:
> Powerpc 44x uses 36 bit real address while the real address defined
>     in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
>     fails to initialize. This fix changes the data types representing the real
>     address from unsigned long 32-bit types to resource_size_t which is 64-bit. The
>     driver has been tested, the disks get discovered correctly and can do IO.
> 
> Signed-off-by: Pravin Bathija <pbathija@amcc.com>
> Acked-by: Feng Kan <fkan@amcc.com>
> Acked-by: Fushen Chen <fchen@amcc.com>
> Acked-by: Loc Ho <lho@amcc.com>
> Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
> Acked-by: Victor Gallardo <vgallardo@amcc.com>
> ---
>  drivers/message/fusion/mptbase.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> index 5d496a9..9f14a60 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -1511,7 +1511,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
>  {
>  	u8		__iomem *mem;
>  	int		 ii;
> -	unsigned long	 mem_phys;
> +	resource_size_t	 mem_phys;

You never actually compiled this, did you?

drivers/message/fusion/mptbase.c: In function 'mpt_mapresources':
drivers/message/fusion/mptbase.c:1680: warning: format '%lx' expects type 'long unsigned int', but argument 4 has type 'resource_size_t'

I'll just fold the fix in

James

---

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 162923f..85bc6a6 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1677,8 +1677,8 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 		return -EINVAL;
 	}
 	ioc->memmap = mem;
-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
-	    ioc->name, mem, mem_phys));
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
+	    ioc->name, mem, (unsigned long long)mem_phys));
 
 	ioc->mem_phys = mem_phys;
 	ioc->chip = (SYSIF_REGS __iomem *)mem;

RE: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Benjamin Herrenschmidt]

On Thu, 2009-12-03 at 15:21 -0800, Pravin Bathija wrote:
> Hi Wolfgang,

 .../...
 
> > I'm not sure if this one-liner really covers all the related issues.
> > We submitted a similar (but apparently more complete) patch more than
> > a year ago. Dunno why it has never been picked up. See
> > http://thread.gmane.org/gmane.linux.scsi/46082 for reference.
> > 
> 
> I submitted a patch on similar lines several weeks ago and it wasn't
> accepted on grounds that it was too tied to the powerpc platform. Below
> is a link
> 
> http://article.gmane.org/gmane.linux.scsi/55794

I believe the simple patch is fine. But only testing can tell, so it's
up to you guys to test it :-)

None of the churn related to PIO that was in the previous patches is
necessary.

PIO on powerpc "appears" to work just like x86, the illusion is
maintained by the arch code. PIO resources always fit inside 32-bit,
never need to be ioremapped etc... so as long as you use the result of
pci_resource_start() and pass that (or an offset from that) to
inb/outb/intw/outw... PIO should just work, no change is required to the
driver. IE. PIO resources don't contain physical addresses. The powerpc
PCI code puts in there an offset from _IO_BASE to an already ioremapped
area mapping the PCI host bridge IO space. It can use funky pointer
arithmetic so don't be surprised if the values look like negative 32-bit
ints, but it should just work.

The only problem I can see with the driver, which is fixed by the simple
patch, is that for -MMIO-, the resources (which in the case of MMIO do
contain physical addresses) can be >32 bit, and thus must be stored into
a type of the right size before being passed to ioremap(). This is what
the one-liner patch does and according to the patch author, it was
tested and appears to work.

So I'm happy with the one liner patch. If you have more concerns, please
explain precisely what you believe will not work :-)

Cheers,
Ben.

RE: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Benjamin Herrenschmidt]

On Thu, 2009-12-03 at 15:21 -0800, Pravin Bathija wrote:
> Hi Wolfgang,

 .../...
 
> > I'm not sure if this one-liner really covers all the related issues.
> > We submitted a similar (but apparently more complete) patch more than
> > a year ago. Dunno why it has never been picked up. See
> > http://thread.gmane.org/gmane.linux.scsi/46082 for reference.
> > 
> 
> I submitted a patch on similar lines several weeks ago and it wasn't
> accepted on grounds that it was too tied to the powerpc platform. Below
> is a link
> 
> http://article.gmane.org/gmane.linux.scsi/55794

I believe the simple patch is fine. But only testing can tell, so it's
up to you guys to test it :-)

None of the churn related to PIO that was in the previous patches is
necessary.

PIO on powerpc "appears" to work just like x86, the illusion is
maintained by the arch code. PIO resources always fit inside 32-bit,
never need to be ioremapped etc... so as long as you use the result of
pci_resource_start() and pass that (or an offset from that) to
inb/outb/intw/outw... PIO should just work, no change is required to the
driver. IE. PIO resources don't contain physical addresses. The powerpc
PCI code puts in there an offset from _IO_BASE to an already ioremapped
area mapping the PCI host bridge IO space. It can use funky pointer
arithmetic so don't be surprised if the values look like negative 32-bit
ints, but it should just work.

The only problem I can see with the driver, which is fixed by the simple
patch, is that for -MMIO-, the resources (which in the case of MMIO do
contain physical addresses) can be >32 bit, and thus must be stored into
a type of the right size before being passed to ioremap(). This is what
the one-liner patch does and according to the patch author, it was
tested and appears to work.

So I'm happy with the one liner patch. If you have more concerns, please
explain precisely what you believe will not work :-)

Cheers,
Ben.

RE: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Pravin Bathija]

Hi Wolfgang,

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd@denx.de]
> Sent: Thursday, December 03, 2009 12:56 AM
> To: Pravin Bathija; Benjamin Herrenschmidt; Desai, Kashyap
> Cc: linux-scsi@vger.kernel.org; linuxppc-dev@ozlabs.org;
> Eric.Moore@lsi.com
> Subject: Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64
> bit resources.
> 
> Dear Pravin Bathija,
> 
> In message <1259805106-23636-1-git-send-email-pbathija@amcc.com> you
> wrote:
> >     Powerpc 44x uses 36 bit real address while the real address
> defined
> >     in MPT Fusion driver is of type 32 bit. This causes ioremap to
> fail and driver
> >     fails to initialize. This fix changes the data types
representing
> the real
> >     address from unsigned long 32-bit types to resource_size_t which
> is 64-bit. The
> >     driver has been tested, the disks get discovered correctly and
> can do IO.
> ...
> > --- a/drivers/message/fusion/mptbase.c
> > +++ b/drivers/message/fusion/mptbase.c
> > @@ -1511,7 +1511,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> >  {
> >  	u8		__iomem *mem;
> >  	int		 ii;
> > -	unsigned long	 mem_phys;
> > +	resource_size_t	 mem_phys;
> >  	unsigned long	 port;
> >  	u32		 msize;
> >  	u32		 psize;
> 
> I'm not sure if this one-liner really covers all the related issues.
> We submitted a similar (but apparently more complete) patch more than
> a year ago. Dunno why it has never been picked up. See
> http://thread.gmane.org/gmane.linux.scsi/46082 for reference.
> 

I submitted a patch on similar lines several weeks ago and it wasn't
accepted on grounds that it was too tied to the powerpc platform. Below
is a link

http://article.gmane.org/gmane.linux.scsi/55794




> 
> > From: Yuri Tikhonov <yur@emcraft.com>
> To: linux-scsi@vger.kernel.org
> Subject: [PATCH] mptbase: use resource_size_t instead of unsigned long
> and u32
> Date: Thu, 13 Nov 2008 11:33:16 +0300
> 
>  Hello,
> 
>  The following patch adds using resource_size_t for the
> pci_resource_start()/pci_resource_len() return values. This
> makes mptbase driver work correctly on 32 bit systems with
> 64 bit resources (e.g. PPC440SPe).
> 
> Do some minor cleanups in mpt_mapresources() as well.
> 
> Signed-off-by: Yuri Tikhonov <yur@emcraft.com>
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> ---
>  drivers/message/fusion/mptbase.c |   38
++++++++++++++++++++++++++----
> --------
>  drivers/message/fusion/mptbase.h |    5 +++--
>  2 files changed, 29 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/message/fusion/mptbase.c
> b/drivers/message/fusion/mptbase.c
> index d6a0074..9daf844 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -1488,11 +1488,12 @@ static int
>  mpt_mapresources(MPT_ADAPTER *ioc)
>  {
>  	u8		__iomem *mem;
> +	u8		__iomem *port;
>  	int		 ii;
> -	unsigned long	 mem_phys;
> -	unsigned long	 port;
> -	u32		 msize;
> -	u32		 psize;
> +	resource_size_t	 mem_phys;
> +	resource_size_t	 port_phys;
> +	resource_size_t	 msize;
> +	resource_size_t	 psize;
>  	u8		 revision;
>  	int		 r = -ENODEV;
>  	struct pci_dev *pdev;
> @@ -1530,13 +1531,13 @@ mpt_mapresources(MPT_ADAPTER *ioc)
>  	}
> 
>  	mem_phys = msize = 0;
> -	port = psize = 0;
> +	port_phys = psize = 0;
>  	for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) {
>  		if (pci_resource_flags(pdev, ii) &
> PCI_BASE_ADDRESS_SPACE_IO) {
>  			if (psize)
>  				continue;
>  			/* Get I/O space! */
> -			port = pci_resource_start(pdev, ii);
> +			port_phys = pci_resource_start(pdev, ii);
>  			psize = pci_resource_len(pdev, ii);
>  		} else {
>  			if (msize)
> @@ -1546,11 +1547,8 @@ mpt_mapresources(MPT_ADAPTER *ioc)
>  			msize = pci_resource_len(pdev, ii);
>  		}
>  	}
> -	ioc->mem_size = msize;
> 
> -	mem = NULL;
>  	/* Get logical ptr for PciMem0 space */
> -	/*mem = ioremap(mem_phys, msize);*/
>  	mem = ioremap(mem_phys, msize);
>  	if (mem == NULL) {
>  		printk(MYIOC_s_ERR_FMT ": ERROR - Unable to map adapter"
> @@ -1558,14 +1556,24 @@ mpt_mapresources(MPT_ADAPTER *ioc)
>  		return -EINVAL;
>  	}
>  	ioc->memmap = mem;
> -	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys =
> %lx\n",
> -	    ioc->name, mem, mem_phys));
> +	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem=%p,
> mem_phys=%llx\n",
> +	    ioc->name, mem, (u64)mem_phys));
> 
>  	ioc->mem_phys = mem_phys;
>  	ioc->chip = (SYSIF_REGS __iomem *)mem;
> 
>  	/* Save Port IO values in case we need to do downloadboot */
> -	ioc->pio_mem_phys = port;
> +	port = ioremap(port_phys, psize);
> +	if (port == NULL) {
> +		printk(MYIOC_s_ERR_FMT ": ERROR - Unable to map adapter"
> +			" port!\n", ioc->name);
> +		return -EINVAL;
> +	}
> +	ioc->portmap = port;
> +	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "port=%p,
> port_phys=%llx\n",
> +	    ioc->name, port, (u64)port_phys));
> +
> +	ioc->pio_mem_phys = port_phys;
>  	ioc->pio_chip = (SYSIF_REGS __iomem *)port;
> 
>  	return 0;
> @@ -1790,6 +1798,7 @@ mpt_attach(struct pci_dev *pdev, const struct
> pci_device_id *id)
>  		list_del(&ioc->list);
>  		if (ioc->alt_ioc)
>  			ioc->alt_ioc->alt_ioc = NULL;
> +		iounmap(ioc->portmap);
>  		iounmap(ioc->memmap);
>  		if (r != -5)
>  			pci_release_selected_regions(pdev, ioc->bars);
> @@ -2547,6 +2556,11 @@ mpt_adapter_dispose(MPT_ADAPTER *ioc)
>  		ioc->pci_irq = -1;
>  	}
> 
> +	if (ioc->portmap != NULL) {
> +		iounmap(ioc->portmap);
> +		ioc->portmap = NULL;
> +	}
> +
>  	if (ioc->memmap != NULL) {
>  		iounmap(ioc->memmap);
>  		ioc->memmap = NULL;
> diff --git a/drivers/message/fusion/mptbase.h
> b/drivers/message/fusion/mptbase.h
> index dff048c..17826b3 100644
> --- a/drivers/message/fusion/mptbase.h
> +++ b/drivers/message/fusion/mptbase.h
> @@ -584,8 +584,8 @@ typedef struct _MPT_ADAPTER
>  	SYSIF_REGS __iomem	*chip;		/* == c8817000 (mmap)
> */
>  	SYSIF_REGS __iomem	*pio_chip;	/* Programmed IO
> (downloadboot) */
>  	u8			 bus_type;
> -	u32			 mem_phys;	/* == f4020000 (mmap) */
> -	u32			 pio_mem_phys;	/* Programmed IO
> (downloadboot) */
> +	resource_size_t		 mem_phys;	/* == f4020000 (mmap) */
> +	resource_size_t		 pio_mem_phys;	/* Programmed IO
> (downloadboot) */
>  	int			 mem_size;	/* mmap memory size */
>  	int			 number_of_buses;
>  	int			 devices_per_bus;
> @@ -635,6 +635,7 @@ typedef struct _MPT_ADAPTER
>  	int			bars;		/* bitmask of BAR's that
must be
> configured */
>  	int			msi_enable;
>  	u8			__iomem *memmap;	/* mmap address
*/
> +	u8			__iomem *portmap;	/* mmap address
*/
>  	struct Scsi_Host	*sh;		/* Scsi Host pointer */
>  	SpiCfgData		spi_data;	/* Scsi config. data */
>  	RaidCfgData		raid_data;	/* Raid config. data */
> --
> 1.5.6.1
> 
> 
> --
> Yuri Tikhonov, Senior Software Engineer
> Emcraft Systems, www.emcraft.com
> 
> 
> 
> 
> 
> 
> Best regards,
> 
> Wolfgang Denk
> 
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> "I haven't lost my mind - it's backed up on tape somewhere."

RE: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Pravin Bathija]

Hi Wolfgang,

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd@denx.de]
> Sent: Thursday, December 03, 2009 12:56 AM
> To: Pravin Bathija; Benjamin Herrenschmidt; Desai, Kashyap
> Cc: linux-scsi@vger.kernel.org; linuxppc-dev@ozlabs.org;
> Eric.Moore@lsi.com
> Subject: Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64
> bit resources.
>=20
> Dear Pravin Bathija,
>=20
> In message <1259805106-23636-1-git-send-email-pbathija@amcc.com> you
> wrote:
> >     Powerpc 44x uses 36 bit real address while the real address
> defined
> >     in MPT Fusion driver is of type 32 bit. This causes ioremap to
> fail and driver
> >     fails to initialize. This fix changes the data types
representing
> the real
> >     address from unsigned long 32-bit types to resource_size_t which
> is 64-bit. The
> >     driver has been tested, the disks get discovered correctly and
> can do IO.
> ...
> > --- a/drivers/message/fusion/mptbase.c
> > +++ b/drivers/message/fusion/mptbase.c
> > @@ -1511,7 +1511,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> >  {
> >  	u8		__iomem *mem;
> >  	int		 ii;
> > -	unsigned long	 mem_phys;
> > +	resource_size_t	 mem_phys;
> >  	unsigned long	 port;
> >  	u32		 msize;
> >  	u32		 psize;
>=20
> I'm not sure if this one-liner really covers all the related issues.
> We submitted a similar (but apparently more complete) patch more than
> a year ago. Dunno why it has never been picked up. See
> http://thread.gmane.org/gmane.linux.scsi/46082 for reference.
>=20

I submitted a patch on similar lines several weeks ago and it wasn't
accepted on grounds that it was too tied to the powerpc platform. Below
is a link

http://article.gmane.org/gmane.linux.scsi/55794




>=20
> > From: Yuri Tikhonov <yur@emcraft.com>
> To: linux-scsi@vger.kernel.org
> Subject: [PATCH] mptbase: use resource_size_t instead of unsigned long
> and u32
> Date: Thu, 13 Nov 2008 11:33:16 +0300
>=20
>  Hello,
>=20
>  The following patch adds using resource_size_t for the
> pci_resource_start()/pci_resource_len() return values. This
> makes mptbase driver work correctly on 32 bit systems with
> 64 bit resources (e.g. PPC440SPe).
>=20
> Do some minor cleanups in mpt_mapresources() as well.
>=20
> Signed-off-by: Yuri Tikhonov <yur@emcraft.com>
> Signed-off-by: Ilya Yanok <yanok@emcraft.com>
> ---
>  drivers/message/fusion/mptbase.c |   38
++++++++++++++++++++++++++----
> --------
>  drivers/message/fusion/mptbase.h |    5 +++--
>  2 files changed, 29 insertions(+), 14 deletions(-)
>=20
> diff --git a/drivers/message/fusion/mptbase.c
> b/drivers/message/fusion/mptbase.c
> index d6a0074..9daf844 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -1488,11 +1488,12 @@ static int
>  mpt_mapresources(MPT_ADAPTER *ioc)
>  {
>  	u8		__iomem *mem;
> +	u8		__iomem *port;
>  	int		 ii;
> -	unsigned long	 mem_phys;
> -	unsigned long	 port;
> -	u32		 msize;
> -	u32		 psize;
> +	resource_size_t	 mem_phys;
> +	resource_size_t	 port_phys;
> +	resource_size_t	 msize;
> +	resource_size_t	 psize;
>  	u8		 revision;
>  	int		 r =3D -ENODEV;
>  	struct pci_dev *pdev;
> @@ -1530,13 +1531,13 @@ mpt_mapresources(MPT_ADAPTER *ioc)
>  	}
>=20
>  	mem_phys =3D msize =3D 0;
> -	port =3D psize =3D 0;
> +	port_phys =3D psize =3D 0;
>  	for (ii =3D 0; ii < DEVICE_COUNT_RESOURCE; ii++) {
>  		if (pci_resource_flags(pdev, ii) &
> PCI_BASE_ADDRESS_SPACE_IO) {
>  			if (psize)
>  				continue;
>  			/* Get I/O space! */
> -			port =3D pci_resource_start(pdev, ii);
> +			port_phys =3D pci_resource_start(pdev, ii);
>  			psize =3D pci_resource_len(pdev, ii);
>  		} else {
>  			if (msize)
> @@ -1546,11 +1547,8 @@ mpt_mapresources(MPT_ADAPTER *ioc)
>  			msize =3D pci_resource_len(pdev, ii);
>  		}
>  	}
> -	ioc->mem_size =3D msize;
>=20
> -	mem =3D NULL;
>  	/* Get logical ptr for PciMem0 space */
> -	/*mem =3D ioremap(mem_phys, msize);*/
>  	mem =3D ioremap(mem_phys, msize);
>  	if (mem =3D=3D NULL) {
>  		printk(MYIOC_s_ERR_FMT ": ERROR - Unable to map adapter"
> @@ -1558,14 +1556,24 @@ mpt_mapresources(MPT_ADAPTER *ioc)
>  		return -EINVAL;
>  	}
>  	ioc->memmap =3D mem;
> -	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem =3D %p, mem_phys =3D
> %lx\n",
> -	    ioc->name, mem, mem_phys));
> +	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem=3D%p,
> mem_phys=3D%llx\n",
> +	    ioc->name, mem, (u64)mem_phys));
>=20
>  	ioc->mem_phys =3D mem_phys;
>  	ioc->chip =3D (SYSIF_REGS __iomem *)mem;
>=20
>  	/* Save Port IO values in case we need to do downloadboot */
> -	ioc->pio_mem_phys =3D port;
> +	port =3D ioremap(port_phys, psize);
> +	if (port =3D=3D NULL) {
> +		printk(MYIOC_s_ERR_FMT ": ERROR - Unable to map adapter"
> +			" port!\n", ioc->name);
> +		return -EINVAL;
> +	}
> +	ioc->portmap =3D port;
> +	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "port=3D%p,
> port_phys=3D%llx\n",
> +	    ioc->name, port, (u64)port_phys));
> +
> +	ioc->pio_mem_phys =3D port_phys;
>  	ioc->pio_chip =3D (SYSIF_REGS __iomem *)port;
>=20
>  	return 0;
> @@ -1790,6 +1798,7 @@ mpt_attach(struct pci_dev *pdev, const struct
> pci_device_id *id)
>  		list_del(&ioc->list);
>  		if (ioc->alt_ioc)
>  			ioc->alt_ioc->alt_ioc =3D NULL;
> +		iounmap(ioc->portmap);
>  		iounmap(ioc->memmap);
>  		if (r !=3D -5)
>  			pci_release_selected_regions(pdev, ioc->bars);
> @@ -2547,6 +2556,11 @@ mpt_adapter_dispose(MPT_ADAPTER *ioc)
>  		ioc->pci_irq =3D -1;
>  	}
>=20
> +	if (ioc->portmap !=3D NULL) {
> +		iounmap(ioc->portmap);
> +		ioc->portmap =3D NULL;
> +	}
> +
>  	if (ioc->memmap !=3D NULL) {
>  		iounmap(ioc->memmap);
>  		ioc->memmap =3D NULL;
> diff --git a/drivers/message/fusion/mptbase.h
> b/drivers/message/fusion/mptbase.h
> index dff048c..17826b3 100644
> --- a/drivers/message/fusion/mptbase.h
> +++ b/drivers/message/fusion/mptbase.h
> @@ -584,8 +584,8 @@ typedef struct _MPT_ADAPTER
>  	SYSIF_REGS __iomem	*chip;		/* =3D=3D c8817000 (mmap)
> */
>  	SYSIF_REGS __iomem	*pio_chip;	/* Programmed IO
> (downloadboot) */
>  	u8			 bus_type;
> -	u32			 mem_phys;	/* =3D=3D f4020000 (mmap) */
> -	u32			 pio_mem_phys;	/* Programmed IO
> (downloadboot) */
> +	resource_size_t		 mem_phys;	/* =3D=3D f4020000 (mmap) */
> +	resource_size_t		 pio_mem_phys;	/* Programmed IO
> (downloadboot) */
>  	int			 mem_size;	/* mmap memory size */
>  	int			 number_of_buses;
>  	int			 devices_per_bus;
> @@ -635,6 +635,7 @@ typedef struct _MPT_ADAPTER
>  	int			bars;		/* bitmask of BAR's that
must be
> configured */
>  	int			msi_enable;
>  	u8			__iomem *memmap;	/* mmap address
*/
> +	u8			__iomem *portmap;	/* mmap address
*/
>  	struct Scsi_Host	*sh;		/* Scsi Host pointer */
>  	SpiCfgData		spi_data;	/* Scsi config. data */
>  	RaidCfgData		raid_data;	/* Raid config. data */
> --
> 1.5.6.1
>=20
>=20
> --
> Yuri Tikhonov, Senior Software Engineer
> Emcraft Systems, www.emcraft.com
>=20
>=20
>=20
>=20
>=20
>=20
> Best regards,
>=20
> Wolfgang Denk
>=20
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
> "I haven't lost my mind - it's backed up on tape somewhere."

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Wolfgang Denk]

Dear Pravin Bathija,

In message <1259805106-23636-1-git-send-email-pbathija@amcc.com> you wrote:
>     Powerpc 44x uses 36 bit real address while the real address defined
>     in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
>     fails to initialize. This fix changes the data types representing the real
>     address from unsigned long 32-bit types to resource_size_t which is 64-bit. The
>     driver has been tested, the disks get discovered correctly and can do IO.
...
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -1511,7 +1511,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
>  {
>  	u8		__iomem *mem;
>  	int		 ii;
> -	unsigned long	 mem_phys;
> +	resource_size_t	 mem_phys;
>  	unsigned long	 port;
>  	u32		 msize;
>  	u32		 psize;

I'm not sure if this one-liner really covers all the related issues.
We submitted a similar (but apparently more complete) patch more than
a year ago. Dunno why it has never been picked up. See
http://thread.gmane.org/gmane.linux.scsi/46082 for reference.


> From: Yuri Tikhonov <yur@emcraft.com>
To: linux-scsi@vger.kernel.org
Subject: [PATCH] mptbase: use resource_size_t instead of unsigned long and u32
Date: Thu, 13 Nov 2008 11:33:16 +0300

 Hello,

 The following patch adds using resource_size_t for the
pci_resource_start()/pci_resource_len() return values. This
makes mptbase driver work correctly on 32 bit systems with
64 bit resources (e.g. PPC440SPe).

Do some minor cleanups in mpt_mapresources() as well.

Signed-off-by: Yuri Tikhonov <yur@emcraft.com>
Signed-off-by: Ilya Yanok <yanok@emcraft.com>
---
 drivers/message/fusion/mptbase.c |   38 ++++++++++++++++++++++++++------------
 drivers/message/fusion/mptbase.h |    5 +++--
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index d6a0074..9daf844 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1488,11 +1488,12 @@ static int
 mpt_mapresources(MPT_ADAPTER *ioc)
 {
 	u8		__iomem *mem;
+	u8		__iomem *port;
 	int		 ii;
-	unsigned long	 mem_phys;
-	unsigned long	 port;
-	u32		 msize;
-	u32		 psize;
+	resource_size_t	 mem_phys;
+	resource_size_t	 port_phys;
+	resource_size_t	 msize;
+	resource_size_t	 psize;
 	u8		 revision;
 	int		 r = -ENODEV;
 	struct pci_dev *pdev;
@@ -1530,13 +1531,13 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 	}
 
 	mem_phys = msize = 0;
-	port = psize = 0;
+	port_phys = psize = 0;
 	for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) {
 		if (pci_resource_flags(pdev, ii) & PCI_BASE_ADDRESS_SPACE_IO) {
 			if (psize)
 				continue;
 			/* Get I/O space! */
-			port = pci_resource_start(pdev, ii);
+			port_phys = pci_resource_start(pdev, ii);
 			psize = pci_resource_len(pdev, ii);
 		} else {
 			if (msize)
@@ -1546,11 +1547,8 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 			msize = pci_resource_len(pdev, ii);
 		}
 	}
-	ioc->mem_size = msize;
 
-	mem = NULL;
 	/* Get logical ptr for PciMem0 space */
-	/*mem = ioremap(mem_phys, msize);*/
 	mem = ioremap(mem_phys, msize);
 	if (mem == NULL) {
 		printk(MYIOC_s_ERR_FMT ": ERROR - Unable to map adapter"
@@ -1558,14 +1556,24 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 		return -EINVAL;
 	}
 	ioc->memmap = mem;
-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
-	    ioc->name, mem, mem_phys));
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem=%p, mem_phys=%llx\n",
+	    ioc->name, mem, (u64)mem_phys));
 
 	ioc->mem_phys = mem_phys;
 	ioc->chip = (SYSIF_REGS __iomem *)mem;
 
 	/* Save Port IO values in case we need to do downloadboot */
-	ioc->pio_mem_phys = port;
+	port = ioremap(port_phys, psize);
+	if (port == NULL) {
+		printk(MYIOC_s_ERR_FMT ": ERROR - Unable to map adapter"
+			" port!\n", ioc->name);
+		return -EINVAL;
+	}
+	ioc->portmap = port;
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "port=%p, port_phys=%llx\n",
+	    ioc->name, port, (u64)port_phys));
+
+	ioc->pio_mem_phys = port_phys;
 	ioc->pio_chip = (SYSIF_REGS __iomem *)port;
 
 	return 0;
@@ -1790,6 +1798,7 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 		list_del(&ioc->list);
 		if (ioc->alt_ioc)
 			ioc->alt_ioc->alt_ioc = NULL;
+		iounmap(ioc->portmap);
 		iounmap(ioc->memmap);
 		if (r != -5)
 			pci_release_selected_regions(pdev, ioc->bars);
@@ -2547,6 +2556,11 @@ mpt_adapter_dispose(MPT_ADAPTER *ioc)
 		ioc->pci_irq = -1;
 	}
 
+	if (ioc->portmap != NULL) {
+		iounmap(ioc->portmap);
+		ioc->portmap = NULL;
+	}
+
 	if (ioc->memmap != NULL) {
 		iounmap(ioc->memmap);
 		ioc->memmap = NULL;
diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index dff048c..17826b3 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -584,8 +584,8 @@ typedef struct _MPT_ADAPTER
 	SYSIF_REGS __iomem	*chip;		/* == c8817000 (mmap) */
 	SYSIF_REGS __iomem	*pio_chip;	/* Programmed IO (downloadboot) */
 	u8			 bus_type;
-	u32			 mem_phys;	/* == f4020000 (mmap) */
-	u32			 pio_mem_phys;	/* Programmed IO (downloadboot) */
+	resource_size_t		 mem_phys;	/* == f4020000 (mmap) */
+	resource_size_t		 pio_mem_phys;	/* Programmed IO (downloadboot) */
 	int			 mem_size;	/* mmap memory size */
 	int			 number_of_buses;
 	int			 devices_per_bus;
@@ -635,6 +635,7 @@ typedef struct _MPT_ADAPTER
 	int			bars;		/* bitmask of BAR's that must be configured */
 	int			msi_enable;
 	u8			__iomem *memmap;	/* mmap address */
+	u8			__iomem *portmap;	/* mmap address */
 	struct Scsi_Host	*sh;		/* Scsi Host pointer */
 	SpiCfgData		spi_data;	/* Scsi config. data */
 	RaidCfgData		raid_data;	/* Raid config. data */
-- 
1.5.6.1


-- 
Yuri Tikhonov, Senior Software Engineer
Emcraft Systems, www.emcraft.com






Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
"I haven't lost my mind - it's backed up on tape somewhere."

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Wolfgang Denk]

Dear Pravin Bathija,

In message <1259805106-23636-1-git-send-email-pbathija@amcc.com> you wrote:
>     Powerpc 44x uses 36 bit real address while the real address defined
>     in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
>     fails to initialize. This fix changes the data types representing the real
>     address from unsigned long 32-bit types to resource_size_t which is 64-bit. The
>     driver has been tested, the disks get discovered correctly and can do IO.
...
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -1511,7 +1511,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
>  {
>  	u8		__iomem *mem;
>  	int		 ii;
> -	unsigned long	 mem_phys;
> +	resource_size_t	 mem_phys;
>  	unsigned long	 port;
>  	u32		 msize;
>  	u32		 psize;

I'm not sure if this one-liner really covers all the related issues.
We submitted a similar (but apparently more complete) patch more than
a year ago. Dunno why it has never been picked up. See
http://thread.gmane.org/gmane.linux.scsi/46082 for reference.


> From: Yuri Tikhonov <yur@emcraft.com>
To: linux-scsi@vger.kernel.org
Subject: [PATCH] mptbase: use resource_size_t instead of unsigned long and u32
Date: Thu, 13 Nov 2008 11:33:16 +0300

 Hello,

 The following patch adds using resource_size_t for the
pci_resource_start()/pci_resource_len() return values. This
makes mptbase driver work correctly on 32 bit systems with
64 bit resources (e.g. PPC440SPe).

Do some minor cleanups in mpt_mapresources() as well.

Signed-off-by: Yuri Tikhonov <yur@emcraft.com>
Signed-off-by: Ilya Yanok <yanok@emcraft.com>
---
 drivers/message/fusion/mptbase.c |   38 ++++++++++++++++++++++++++------------
 drivers/message/fusion/mptbase.h |    5 +++--
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index d6a0074..9daf844 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1488,11 +1488,12 @@ static int
 mpt_mapresources(MPT_ADAPTER *ioc)
 {
 	u8		__iomem *mem;
+	u8		__iomem *port;
 	int		 ii;
-	unsigned long	 mem_phys;
-	unsigned long	 port;
-	u32		 msize;
-	u32		 psize;
+	resource_size_t	 mem_phys;
+	resource_size_t	 port_phys;
+	resource_size_t	 msize;
+	resource_size_t	 psize;
 	u8		 revision;
 	int		 r = -ENODEV;
 	struct pci_dev *pdev;
@@ -1530,13 +1531,13 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 	}
 
 	mem_phys = msize = 0;
-	port = psize = 0;
+	port_phys = psize = 0;
 	for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) {
 		if (pci_resource_flags(pdev, ii) & PCI_BASE_ADDRESS_SPACE_IO) {
 			if (psize)
 				continue;
 			/* Get I/O space! */
-			port = pci_resource_start(pdev, ii);
+			port_phys = pci_resource_start(pdev, ii);
 			psize = pci_resource_len(pdev, ii);
 		} else {
 			if (msize)
@@ -1546,11 +1547,8 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 			msize = pci_resource_len(pdev, ii);
 		}
 	}
-	ioc->mem_size = msize;
 
-	mem = NULL;
 	/* Get logical ptr for PciMem0 space */
-	/*mem = ioremap(mem_phys, msize);*/
 	mem = ioremap(mem_phys, msize);
 	if (mem == NULL) {
 		printk(MYIOC_s_ERR_FMT ": ERROR - Unable to map adapter"
@@ -1558,14 +1556,24 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 		return -EINVAL;
 	}
 	ioc->memmap = mem;
-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
-	    ioc->name, mem, mem_phys));
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem=%p, mem_phys=%llx\n",
+	    ioc->name, mem, (u64)mem_phys));
 
 	ioc->mem_phys = mem_phys;
 	ioc->chip = (SYSIF_REGS __iomem *)mem;
 
 	/* Save Port IO values in case we need to do downloadboot */
-	ioc->pio_mem_phys = port;
+	port = ioremap(port_phys, psize);
+	if (port == NULL) {
+		printk(MYIOC_s_ERR_FMT ": ERROR - Unable to map adapter"
+			" port!\n", ioc->name);
+		return -EINVAL;
+	}
+	ioc->portmap = port;
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "port=%p, port_phys=%llx\n",
+	    ioc->name, port, (u64)port_phys));
+
+	ioc->pio_mem_phys = port_phys;
 	ioc->pio_chip = (SYSIF_REGS __iomem *)port;
 
 	return 0;
@@ -1790,6 +1798,7 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 		list_del(&ioc->list);
 		if (ioc->alt_ioc)
 			ioc->alt_ioc->alt_ioc = NULL;
+		iounmap(ioc->portmap);
 		iounmap(ioc->memmap);
 		if (r != -5)
 			pci_release_selected_regions(pdev, ioc->bars);
@@ -2547,6 +2556,11 @@ mpt_adapter_dispose(MPT_ADAPTER *ioc)
 		ioc->pci_irq = -1;
 	}
 
+	if (ioc->portmap != NULL) {
+		iounmap(ioc->portmap);
+		ioc->portmap = NULL;
+	}
+
 	if (ioc->memmap != NULL) {
 		iounmap(ioc->memmap);
 		ioc->memmap = NULL;
diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index dff048c..17826b3 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -584,8 +584,8 @@ typedef struct _MPT_ADAPTER
 	SYSIF_REGS __iomem	*chip;		/* == c8817000 (mmap) */
 	SYSIF_REGS __iomem	*pio_chip;	/* Programmed IO (downloadboot) */
 	u8			 bus_type;
-	u32			 mem_phys;	/* == f4020000 (mmap) */
-	u32			 pio_mem_phys;	/* Programmed IO (downloadboot) */
+	resource_size_t		 mem_phys;	/* == f4020000 (mmap) */
+	resource_size_t		 pio_mem_phys;	/* Programmed IO (downloadboot) */
 	int			 mem_size;	/* mmap memory size */
 	int			 number_of_buses;
 	int			 devices_per_bus;
@@ -635,6 +635,7 @@ typedef struct _MPT_ADAPTER
 	int			bars;		/* bitmask of BAR's that must be configured */
 	int			msi_enable;
 	u8			__iomem *memmap;	/* mmap address */
+	u8			__iomem *portmap;	/* mmap address */
 	struct Scsi_Host	*sh;		/* Scsi Host pointer */
 	SpiCfgData		spi_data;	/* Scsi config. data */
 	RaidCfgData		raid_data;	/* Raid config. data */
-- 
1.5.6.1


-- 
Yuri Tikhonov, Senior Software Engineer
Emcraft Systems, www.emcraft.com






Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
"I haven't lost my mind - it's backed up on tape somewhere."

RE: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Desai, Kashyap]

This patch looks OK. Please consider it as ACKed by me on behalf of LSI.

- Kashyap

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Benjamin Herrenschmidt
> Sent: Thursday, December 03, 2009 8:30 AM
> To: Pravin Bathija
> Cc: linux-scsi@vger.kernel.org; linuxppc-dev@ozlabs.org;
> jwboyer@linux.vnet.ibm.com; Moore, Eric
> Subject: Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit
> resources.
> 
> On Wed, 2009-12-02 at 17:51 -0800, Pravin Bathija wrote:
> > Powerpc 44x uses 36 bit real address while the real address defined
> >     in MPT Fusion driver is of type 32 bit. This causes ioremap to fail
> and driver
> >     fails to initialize. This fix changes the data types representing
> the real
> >     address from unsigned long 32-bit types to resource_size_t which is
> 64-bit. The
> >     driver has been tested, the disks get discovered correctly and can
> do IO.
> >
> > Signed-off-by: Pravin Bathija <pbathija@amcc.com>
> 
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> James, this one should be good ;-)
> 
> > Acked-by: Feng Kan <fkan@amcc.com>
> > Acked-by: Fushen Chen <fchen@amcc.com>
> > Acked-by: Loc Ho <lho@amcc.com>
> > Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
> > Acked-by: Victor Gallardo <vgallardo@amcc.com>
> > ---
> >  drivers/message/fusion/mptbase.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/message/fusion/mptbase.c
> b/drivers/message/fusion/mptbase.c
> > index 5d496a9..9f14a60 100644
> > --- a/drivers/message/fusion/mptbase.c
> > +++ b/drivers/message/fusion/mptbase.c
> > @@ -1511,7 +1511,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> >  {
> >  	u8		__iomem *mem;
> >  	int		 ii;
> > -	unsigned long	 mem_phys;
> > +	resource_size_t	 mem_phys;
> >  	unsigned long	 port;
> >  	u32		 msize;
> >  	u32		 psize;
> 
> 
> --
> 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] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Desai, Kashyap]

This patch looks OK. Please consider it as ACKed by me on behalf of LSI.

- Kashyap

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Benjamin Herrenschmidt
> Sent: Thursday, December 03, 2009 8:30 AM
> To: Pravin Bathija
> Cc: linux-scsi@vger.kernel.org; linuxppc-dev@ozlabs.org;
> jwboyer@linux.vnet.ibm.com; Moore, Eric
> Subject: Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit
> resources.
>=20
> On Wed, 2009-12-02 at 17:51 -0800, Pravin Bathija wrote:
> > Powerpc 44x uses 36 bit real address while the real address defined
> >     in MPT Fusion driver is of type 32 bit. This causes ioremap to fail
> and driver
> >     fails to initialize. This fix changes the data types representing
> the real
> >     address from unsigned long 32-bit types to resource_size_t which is
> 64-bit. The
> >     driver has been tested, the disks get discovered correctly and can
> do IO.
> >
> > Signed-off-by: Pravin Bathija <pbathija@amcc.com>
>=20
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>=20
> James, this one should be good ;-)
>=20
> > Acked-by: Feng Kan <fkan@amcc.com>
> > Acked-by: Fushen Chen <fchen@amcc.com>
> > Acked-by: Loc Ho <lho@amcc.com>
> > Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
> > Acked-by: Victor Gallardo <vgallardo@amcc.com>
> > ---
> >  drivers/message/fusion/mptbase.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/message/fusion/mptbase.c
> b/drivers/message/fusion/mptbase.c
> > index 5d496a9..9f14a60 100644
> > --- a/drivers/message/fusion/mptbase.c
> > +++ b/drivers/message/fusion/mptbase.c
> > @@ -1511,7 +1511,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> >  {
> >  	u8		__iomem *mem;
> >  	int		 ii;
> > -	unsigned long	 mem_phys;
> > +	resource_size_t	 mem_phys;
> >  	unsigned long	 port;
> >  	u32		 msize;
> >  	u32		 psize;
>=20
>=20
> --
> 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] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Benjamin Herrenschmidt]

On Wed, 2009-12-02 at 17:51 -0800, Pravin Bathija wrote:
> Powerpc 44x uses 36 bit real address while the real address defined
>     in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
>     fails to initialize. This fix changes the data types representing the real
>     address from unsigned long 32-bit types to resource_size_t which is 64-bit. The
>     driver has been tested, the disks get discovered correctly and can do IO.
> 
> Signed-off-by: Pravin Bathija <pbathija@amcc.com>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

James, this one should be good ;-)

> Acked-by: Feng Kan <fkan@amcc.com>
> Acked-by: Fushen Chen <fchen@amcc.com>
> Acked-by: Loc Ho <lho@amcc.com>
> Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
> Acked-by: Victor Gallardo <vgallardo@amcc.com>
> ---
>  drivers/message/fusion/mptbase.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> index 5d496a9..9f14a60 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -1511,7 +1511,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
>  {
>  	u8		__iomem *mem;
>  	int		 ii;
> -	unsigned long	 mem_phys;
> +	resource_size_t	 mem_phys;
>  	unsigned long	 port;
>  	u32		 msize;
>  	u32		 psize;

[PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Pravin Bathija]

    Powerpc 44x uses 36 bit real address while the real address defined
    in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
    fails to initialize. This fix changes the data types representing the real
    address from unsigned long 32-bit types to resource_size_t which is 64-bit. The
    driver has been tested, the disks get discovered correctly and can do IO.

Signed-off-by: Pravin Bathija <pbathija@amcc.com>
Acked-by: Feng Kan <fkan@amcc.com>
Acked-by: Fushen Chen <fchen@amcc.com>
Acked-by: Loc Ho <lho@amcc.com>
Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
Acked-by: Victor Gallardo <vgallardo@amcc.com>
---
 drivers/message/fusion/mptbase.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 5d496a9..9f14a60 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1511,7 +1511,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 {
 	u8		__iomem *mem;
 	int		 ii;
-	unsigned long	 mem_phys;
+	resource_size_t	 mem_phys;
 	unsigned long	 port;
 	u32		 msize;
 	u32		 psize;
-- 
1.5.5

[PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Pravin Bathija]

    Powerpc 44x uses 36 bit real address while the real address defined
    in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
    fails to initialize. This fix changes the data types representing the real
    address from unsigned long 32-bit types to resource_size_t which is 64-bit. The
    driver has been tested, the disks get discovered correctly and can do IO.

Signed-off-by: Pravin Bathija <pbathija@amcc.com>
Acked-by: Feng Kan <fkan@amcc.com>
Acked-by: Fushen Chen <fchen@amcc.com>
Acked-by: Loc Ho <lho@amcc.com>
Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
Acked-by: Victor Gallardo <vgallardo@amcc.com>
---
 drivers/message/fusion/mptbase.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 5d496a9..9f14a60 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1511,7 +1511,7 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 {
 	u8		__iomem *mem;
 	int		 ii;
-	unsigned long	 mem_phys;
+	resource_size_t	 mem_phys;
 	unsigned long	 port;
 	u32		 msize;
 	u32		 psize;
-- 
1.5.5

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Benjamin Herrenschmidt]

On Tue, 2009-11-17 at 16:16 -0800, pbathija@amcc.com wrote:
> From: Pravin Bathija <pbathija@amcc.com>
> 
>   Powerpc 44x uses 36 bit real address while the real address defined
>   in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
>   fails to initialize. This fix changes the data types representing the real
>   address from unsigned long 32-bit types to "phys_addr_t" which is 64-bit. The
>   driver has been tested, the disks get discovered correctly and can do IO. Removed
>   ioremap and used hose->io_base_virt for IO space to make it platform independent.
> Content-Type: text/plain; charset=utf-8

Except that this is all wrong :-) You basically made it powerpc specific
since none of that pci controller stuff is generic. I don't understand
what you are trying to do though. The -only- change you need to do is
to change the longs into resource_size_t.

IO Port numbers are "special" and handled as such already (and besides
are never bigger than 32-bit neither, at least on x86 and powerpc).

Just leave the PIO code alone, hopefully, if the driver isn't full of
crack, the code should be fine already. If the driver does something
wrong then you can attempt to fix it separately.

The only problem that you need to address afaik, is purely that
pci_resource_start() can return a resource_size_t which can be 64-bit,
and as such it is broken for the driver to manipulate and store that
value as an unsigned long or a u32.

Cheers,
Ben.

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Benjamin Herrenschmidt]

On Tue, 2009-11-17 at 16:16 -0800, pbathija@amcc.com wrote:
> From: Pravin Bathija <pbathija@amcc.com>
> 
>   Powerpc 44x uses 36 bit real address while the real address defined
>   in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
>   fails to initialize. This fix changes the data types representing the real
>   address from unsigned long 32-bit types to "phys_addr_t" which is 64-bit. The
>   driver has been tested, the disks get discovered correctly and can do IO. Removed
>   ioremap and used hose->io_base_virt for IO space to make it platform independent.
> Content-Type: text/plain; charset=utf-8

Except that this is all wrong :-) You basically made it powerpc specific
since none of that pci controller stuff is generic. I don't understand
what you are trying to do though. The -only- change you need to do is
to change the longs into resource_size_t.

IO Port numbers are "special" and handled as such already (and besides
are never bigger than 32-bit neither, at least on x86 and powerpc).

Just leave the PIO code alone, hopefully, if the driver isn't full of
crack, the code should be fine already. If the driver does something
wrong then you can attempt to fix it separately.

The only problem that you need to address afaik, is purely that
pci_resource_start() can return a resource_size_t which can be 64-bit,
and as such it is broken for the driver to manipulate and store that
value as an unsigned long or a u32.

Cheers,
Ben.

[PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[pbathija]

From: Pravin Bathija <pbathija@amcc.com>

  Powerpc 44x uses 36 bit real address while the real address defined
  in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
  fails to initialize. This fix changes the data types representing the real
  address from unsigned long 32-bit types to "phys_addr_t" which is 64-bit. The
  driver has been tested, the disks get discovered correctly and can do IO. Removed
  ioremap and used hose->io_base_virt for IO space to make it platform independent.
Content-Type: text/plain; charset=utf-8

Signed-off-by: Pravin Bathija <pbathija@amcc.com>
Acked-by: Feng Kan <fkan@amcc.com>
Acked-by: Prodyut Hazarika <phazarika@amcc.com>
Acked-by: Loc Ho <lho@amcc.com>
Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
Acked-by: Victor Gallardo <vgallardo@amcc.com>
---
 drivers/message/fusion/mptbase.c |   38 ++++++++++++++++++++++++++++----------
 drivers/message/fusion/mptbase.h |    5 +++--
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 5d496a9..9bca4bd 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1510,11 +1510,12 @@ static int
 mpt_mapresources(MPT_ADAPTER *ioc)
 {
 	u8		__iomem *mem;
+	u8		__iomem *port;
 	int		 ii;
-	unsigned long	 mem_phys;
-	unsigned long	 port;
-	u32		 msize;
-	u32		 psize;
+	resource_size_t	 mem_phys;
+	resource_size_t	 port_phys;
+	resource_size_t	 msize;
+	resource_size_t	 psize;
 	u8		 revision;
 	int		 r = -ENODEV;
 	struct pci_dev *pdev;
@@ -1552,14 +1553,24 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 	}
 
 	mem_phys = msize = 0;
-	port = psize = 0;
+	port_phys = psize = 0;
 	for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) {
 		if (pci_resource_flags(pdev, ii) & PCI_BASE_ADDRESS_SPACE_IO) {
 			if (psize)
 				continue;
 			/* Get I/O space! */
-			port = pci_resource_start(pdev, ii);
+			port_phys = pci_resource_start(pdev, ii);
 			psize = pci_resource_len(pdev, ii);
+			struct pci_controller *hose =
+						pci_bus_to_host(pdev->bus);
+			port = hose->io_base_virt;
+			if (port == NULL) {
+				printk(MYIOC_s_ERR_FMT " : ERROR - Unable to"
+					"map adapter port !\n", ioc->name);
+				return -EINVAL;
+			}
+			ioc->pio_mem_phys = port_phys;
+			ioc->pio_chip = (SYSIF_REGS __iomem *)port;
 		} else {
 			if (msize)
 				continue;
@@ -1580,15 +1591,16 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 		return -EINVAL;
 	}
 	ioc->memmap = mem;
-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
-	    ioc->name, mem, mem_phys));
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
+	    ioc->name, mem, (u64)mem_phys));
 
 	ioc->mem_phys = mem_phys;
 	ioc->chip = (SYSIF_REGS __iomem *)mem;
 
 	/* Save Port IO values in case we need to do downloadboot */
-	ioc->pio_mem_phys = port;
-	ioc->pio_chip = (SYSIF_REGS __iomem *)port;
+	ioc->portmap = port;
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "port=%p, port_phys=%llx\n",
+			ioc->name, port, (u64)port_phys));
 
 	return 0;
 }
@@ -1822,6 +1834,7 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 		if (ioc->alt_ioc)
 			ioc->alt_ioc->alt_ioc = NULL;
 		iounmap(ioc->memmap);
+		iounmap(ioc->portmap);
 		if (r != -5)
 			pci_release_selected_regions(pdev, ioc->bars);
 
@@ -2583,6 +2596,11 @@ mpt_adapter_dispose(MPT_ADAPTER *ioc)
 		ioc->memmap = NULL;
 	}
 
+	if (ioc->portmap != NULL) {
+		iounmap(ioc->portmap);
+		ioc->portmap = NULL;
+	}
+
 	pci_disable_device(ioc->pcidev);
 	pci_release_selected_regions(ioc->pcidev, ioc->bars);
 
diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index b3e981d..7091f13 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -584,8 +584,8 @@ typedef struct _MPT_ADAPTER
 	SYSIF_REGS __iomem	*chip;		/* == c8817000 (mmap) */
 	SYSIF_REGS __iomem	*pio_chip;	/* Programmed IO (downloadboot) */
 	u8			 bus_type;
-	u32			 mem_phys;	/* == f4020000 (mmap) */
-	u32			 pio_mem_phys;	/* Programmed IO (downloadboot) */
+	resource_size_t		 mem_phys;	/* == f4020000 (mmap) */
+	resource_size_t		 pio_mem_phys;	/* Programmed IO (downloadboot) */
 	int			 mem_size;	/* mmap memory size */
 	int			 number_of_buses;
 	int			 devices_per_bus;
@@ -635,6 +635,7 @@ typedef struct _MPT_ADAPTER
 	int			bars;		/* bitmask of BAR's that must be configured */
 	int			msi_enable;
 	u8			__iomem *memmap;	/* mmap address */
+	u8			__iomem *portmap;	/* mmap port address */
 	struct Scsi_Host	*sh;		/* Scsi Host pointer */
 	SpiCfgData		spi_data;	/* Scsi config. data */
 	RaidCfgData		raid_data;	/* Raid config. data */
-- 
1.5.5

[PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[pbathija]

From: Pravin Bathija <pbathija@amcc.com>

  Powerpc 44x uses 36 bit real address while the real address defined
  in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
  fails to initialize. This fix changes the data types representing the real
  address from unsigned long 32-bit types to "phys_addr_t" which is 64-bit. The
  driver has been tested, the disks get discovered correctly and can do IO. Removed
  ioremap and used hose->io_base_virt for IO space to make it platform independent.
Content-Type: text/plain; charset=utf-8

Signed-off-by: Pravin Bathija <pbathija@amcc.com>
Acked-by: Feng Kan <fkan@amcc.com>
Acked-by: Prodyut Hazarika <phazarika@amcc.com>
Acked-by: Loc Ho <lho@amcc.com>
Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
Acked-by: Victor Gallardo <vgallardo@amcc.com>
---
 drivers/message/fusion/mptbase.c |   38 ++++++++++++++++++++++++++++----------
 drivers/message/fusion/mptbase.h |    5 +++--
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 5d496a9..9bca4bd 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1510,11 +1510,12 @@ static int
 mpt_mapresources(MPT_ADAPTER *ioc)
 {
 	u8		__iomem *mem;
+	u8		__iomem *port;
 	int		 ii;
-	unsigned long	 mem_phys;
-	unsigned long	 port;
-	u32		 msize;
-	u32		 psize;
+	resource_size_t	 mem_phys;
+	resource_size_t	 port_phys;
+	resource_size_t	 msize;
+	resource_size_t	 psize;
 	u8		 revision;
 	int		 r = -ENODEV;
 	struct pci_dev *pdev;
@@ -1552,14 +1553,24 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 	}
 
 	mem_phys = msize = 0;
-	port = psize = 0;
+	port_phys = psize = 0;
 	for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) {
 		if (pci_resource_flags(pdev, ii) & PCI_BASE_ADDRESS_SPACE_IO) {
 			if (psize)
 				continue;
 			/* Get I/O space! */
-			port = pci_resource_start(pdev, ii);
+			port_phys = pci_resource_start(pdev, ii);
 			psize = pci_resource_len(pdev, ii);
+			struct pci_controller *hose =
+						pci_bus_to_host(pdev->bus);
+			port = hose->io_base_virt;
+			if (port == NULL) {
+				printk(MYIOC_s_ERR_FMT " : ERROR - Unable to"
+					"map adapter port !\n", ioc->name);
+				return -EINVAL;
+			}
+			ioc->pio_mem_phys = port_phys;
+			ioc->pio_chip = (SYSIF_REGS __iomem *)port;
 		} else {
 			if (msize)
 				continue;
@@ -1580,15 +1591,16 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 		return -EINVAL;
 	}
 	ioc->memmap = mem;
-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
-	    ioc->name, mem, mem_phys));
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
+	    ioc->name, mem, (u64)mem_phys));
 
 	ioc->mem_phys = mem_phys;
 	ioc->chip = (SYSIF_REGS __iomem *)mem;
 
 	/* Save Port IO values in case we need to do downloadboot */
-	ioc->pio_mem_phys = port;
-	ioc->pio_chip = (SYSIF_REGS __iomem *)port;
+	ioc->portmap = port;
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "port=%p, port_phys=%llx\n",
+			ioc->name, port, (u64)port_phys));
 
 	return 0;
 }
@@ -1822,6 +1834,7 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 		if (ioc->alt_ioc)
 			ioc->alt_ioc->alt_ioc = NULL;
 		iounmap(ioc->memmap);
+		iounmap(ioc->portmap);
 		if (r != -5)
 			pci_release_selected_regions(pdev, ioc->bars);
 
@@ -2583,6 +2596,11 @@ mpt_adapter_dispose(MPT_ADAPTER *ioc)
 		ioc->memmap = NULL;
 	}
 
+	if (ioc->portmap != NULL) {
+		iounmap(ioc->portmap);
+		ioc->portmap = NULL;
+	}
+
 	pci_disable_device(ioc->pcidev);
 	pci_release_selected_regions(ioc->pcidev, ioc->bars);
 
diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index b3e981d..7091f13 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -584,8 +584,8 @@ typedef struct _MPT_ADAPTER
 	SYSIF_REGS __iomem	*chip;		/* == c8817000 (mmap) */
 	SYSIF_REGS __iomem	*pio_chip;	/* Programmed IO (downloadboot) */
 	u8			 bus_type;
-	u32			 mem_phys;	/* == f4020000 (mmap) */
-	u32			 pio_mem_phys;	/* Programmed IO (downloadboot) */
+	resource_size_t		 mem_phys;	/* == f4020000 (mmap) */
+	resource_size_t		 pio_mem_phys;	/* Programmed IO (downloadboot) */
 	int			 mem_size;	/* mmap memory size */
 	int			 number_of_buses;
 	int			 devices_per_bus;
@@ -635,6 +635,7 @@ typedef struct _MPT_ADAPTER
 	int			bars;		/* bitmask of BAR's that must be configured */
 	int			msi_enable;
 	u8			__iomem *memmap;	/* mmap address */
+	u8			__iomem *portmap;	/* mmap port address */
 	struct Scsi_Host	*sh;		/* Scsi Host pointer */
 	SpiCfgData		spi_data;	/* Scsi config. data */
 	RaidCfgData		raid_data;	/* Raid config. data */
-- 
1.5.5

RE: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Pravin Bathija]

> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
> Sent: Thursday, November 05, 2009 12:00 PM
> To: James Bottomley
> Cc: Josh Boyer; Eric.Moore@lsi.com; Pravin Bathija; linux-
> scsi@vger.kernel.org; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64
> bit resources.
> 
> On Thu, 2009-11-05 at 10:07 -0600, James Bottomley wrote:
> 
> > > > 	ioc->memmap = mem;
> > > >-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p,
> mem_phys = %lx\n",
> > > >-	    ioc->name, mem, mem_phys));
> > > >+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p,
> mem_phys = %llx\n",
> > > >+	    ioc->name, mem, (u64)mem_phys));
> > > >
> > > > 	ioc->mem_phys = mem_phys;
> > > > 	ioc->chip = (SYSIF_REGS __iomem *)mem;
> > > >
> > > > 	/* Save Port IO values in case we need to do downloadboot
> */
> > > >-	ioc->pio_mem_phys = port;
> > > >+	port = ioremap(port_phys, psize);
> > > >+	if (port == NULL) {
> > > >+		printk(MYIOC_s_ERR_FMT " : ERROR - Unable to map
> adapter"
> > > >+			" port !\n", ioc->name);
> > > >+		return -EINVAL;
> >
> > So this looks problematic on a few platforms ... what happens to
> > platforms that have no IO space?  They automatically fail here and it
> > looks like the adapter never attaches.
> 
> Yup, that part of the patch looks wrong.
> 
> However, a mechanical replacement of unsigned long's with
> resource_size_t to hold physical addresses should be fine despite the
> lack of feedback from LSI.
> 
> Pravin, that ioremap definitely seems like it has nothing to do there,
> port IO is already remapped for you by the core PCI code and should
> work
> "as is". Please respin without that change.
> 
> Cheers,
> Ben.
> 

Thanks for the input. Will make the suggested changes and re-submit patch.
Regards,
Pravin

RE: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Pravin Bathija]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmVuamFtaW4gSGVycmVu
c2NobWlkdCBbbWFpbHRvOmJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZ10NCj4gU2VudDogVGh1cnNk
YXksIE5vdmVtYmVyIDA1LCAyMDA5IDEyOjAwIFBNDQo+IFRvOiBKYW1lcyBCb3R0b21sZXkNCj4g
Q2M6IEpvc2ggQm95ZXI7IEVyaWMuTW9vcmVAbHNpLmNvbTsgUHJhdmluIEJhdGhpamE7IGxpbnV4
LQ0KPiBzY3NpQHZnZXIua2VybmVsLm9yZzsgbGludXhwcGMtZGV2QG96bGFicy5vcmcNCj4gU3Vi
amVjdDogUmU6IFtQQVRDSF0gW1NDU0ldIG1wdCBmdXNpb246IEZpeCAzMiBiaXQgcGxhdGZvcm1z
IHdpdGggNjQNCj4gYml0IHJlc291cmNlcy4NCj4gDQo+IE9uIFRodSwgMjAwOS0xMS0wNSBhdCAx
MDowNyAtMDYwMCwgSmFtZXMgQm90dG9tbGV5IHdyb3RlOg0KPiANCj4gPiA+ID4gCWlvYy0+bWVt
bWFwID0gbWVtOw0KPiA+ID4gPi0JZGluaXRwcmludGsoaW9jLCBwcmludGsoTVlJT0Nfc19JTkZP
X0ZNVCAibWVtID0gJXAsDQo+IG1lbV9waHlzID0gJWx4XG4iLA0KPiA+ID4gPi0JICAgIGlvYy0+
bmFtZSwgbWVtLCBtZW1fcGh5cykpOw0KPiA+ID4gPisJZGluaXRwcmludGsoaW9jLCBwcmludGso
TVlJT0Nfc19JTkZPX0ZNVCAibWVtID0gJXAsDQo+IG1lbV9waHlzID0gJWxseFxuIiwNCj4gPiA+
ID4rCSAgICBpb2MtPm5hbWUsIG1lbSwgKHU2NCltZW1fcGh5cykpOw0KPiA+ID4gPg0KPiA+ID4g
PiAJaW9jLT5tZW1fcGh5cyA9IG1lbV9waHlzOw0KPiA+ID4gPiAJaW9jLT5jaGlwID0gKFNZU0lG
X1JFR1MgX19pb21lbSAqKW1lbTsNCj4gPiA+ID4NCj4gPiA+ID4gCS8qIFNhdmUgUG9ydCBJTyB2
YWx1ZXMgaW4gY2FzZSB3ZSBuZWVkIHRvIGRvIGRvd25sb2FkYm9vdA0KPiAqLw0KPiA+ID4gPi0J
aW9jLT5waW9fbWVtX3BoeXMgPSBwb3J0Ow0KPiA+ID4gPisJcG9ydCA9IGlvcmVtYXAocG9ydF9w
aHlzLCBwc2l6ZSk7DQo+ID4gPiA+KwlpZiAocG9ydCA9PSBOVUxMKSB7DQo+ID4gPiA+KwkJcHJp
bnRrKE1ZSU9DX3NfRVJSX0ZNVCAiIDogRVJST1IgLSBVbmFibGUgdG8gbWFwDQo+IGFkYXB0ZXIi
DQo+ID4gPiA+KwkJCSIgcG9ydCAhXG4iLCBpb2MtPm5hbWUpOw0KPiA+ID4gPisJCXJldHVybiAt
RUlOVkFMOw0KPiA+DQo+ID4gU28gdGhpcyBsb29rcyBwcm9ibGVtYXRpYyBvbiBhIGZldyBwbGF0
Zm9ybXMgLi4uIHdoYXQgaGFwcGVucyB0bw0KPiA+IHBsYXRmb3JtcyB0aGF0IGhhdmUgbm8gSU8g
c3BhY2U/ICBUaGV5IGF1dG9tYXRpY2FsbHkgZmFpbCBoZXJlIGFuZCBpdA0KPiA+IGxvb2tzIGxp
a2UgdGhlIGFkYXB0ZXIgbmV2ZXIgYXR0YWNoZXMuDQo+IA0KPiBZdXAsIHRoYXQgcGFydCBvZiB0
aGUgcGF0Y2ggbG9va3Mgd3JvbmcuDQo+IA0KPiBIb3dldmVyLCBhIG1lY2hhbmljYWwgcmVwbGFj
ZW1lbnQgb2YgdW5zaWduZWQgbG9uZydzIHdpdGgNCj4gcmVzb3VyY2Vfc2l6ZV90IHRvIGhvbGQg
cGh5c2ljYWwgYWRkcmVzc2VzIHNob3VsZCBiZSBmaW5lIGRlc3BpdGUgdGhlDQo+IGxhY2sgb2Yg
ZmVlZGJhY2sgZnJvbSBMU0kuDQo+IA0KPiBQcmF2aW4sIHRoYXQgaW9yZW1hcCBkZWZpbml0ZWx5
IHNlZW1zIGxpa2UgaXQgaGFzIG5vdGhpbmcgdG8gZG8gdGhlcmUsDQo+IHBvcnQgSU8gaXMgYWxy
ZWFkeSByZW1hcHBlZCBmb3IgeW91IGJ5IHRoZSBjb3JlIFBDSSBjb2RlIGFuZCBzaG91bGQNCj4g
d29yaw0KPiAiYXMgaXMiLiBQbGVhc2UgcmVzcGluIHdpdGhvdXQgdGhhdCBjaGFuZ2UuDQo+IA0K
PiBDaGVlcnMsDQo+IEJlbi4NCj4gDQoNClRoYW5rcyBmb3IgdGhlIGlucHV0LiBXaWxsIG1ha2Ug
dGhlIHN1Z2dlc3RlZCBjaGFuZ2VzIGFuZCByZS1zdWJtaXQgcGF0Y2guDQpSZWdhcmRzLA0KUHJh
dmluDQo=

RE: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Pravin Bathija]

[-- Attachment #1.1: Type: text/plain, Size: 1789 bytes --]

-----Original Message-----
From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
Sent: Thu 11/5/2009 12:00 PM
To: James Bottomley
Cc: Josh Boyer; Eric.Moore@lsi.com; Pravin Bathija; linux-scsi@vger.kernel.org; linuxppc-dev@ozlabs.org
Subject: Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.
 
On Thu, 2009-11-05 at 10:07 -0600, James Bottomley wrote:

> > > 	ioc->memmap = mem;
> > >-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
> > >-	    ioc->name, mem, mem_phys));
> > >+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
> > >+	    ioc->name, mem, (u64)mem_phys));
> > >
> > > 	ioc->mem_phys = mem_phys;
> > > 	ioc->chip = (SYSIF_REGS __iomem *)mem;
> > >
> > > 	/* Save Port IO values in case we need to do downloadboot */
> > >-	ioc->pio_mem_phys = port;
> > >+	port = ioremap(port_phys, psize);
> > >+	if (port == NULL) {
> > >+		printk(MYIOC_s_ERR_FMT " : ERROR - Unable to map adapter"
> > >+			" port !\n", ioc->name);
> > >+		return -EINVAL;
> 
> So this looks problematic on a few platforms ... what happens to
> platforms that have no IO space?  They automatically fail here and it
> looks like the adapter never attaches.

> Yup, that part of the patch looks wrong.

> However, a mechanical replacement of unsigned long's with
> resource_size_t to hold physical addresses should be fine despite the
> lack of feedback from LSI.

> Pravin, that ioremap definitely seems like it has nothing to do there,
> port IO is already remapped for you by the core PCI code and should work
> "as is". Please respin without that change.

> Cheers,
>Ben.

Thanks for the input. Will make the suggested changes and re-submit the patch.

Regards,
Pravin




[-- Attachment #1.2: Type: text/html, Size: 2991 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

RE: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Pravin Bathija]

[-- Attachment #1: Type: text/plain, Size: 1789 bytes --]

-----Original Message-----
From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]
Sent: Thu 11/5/2009 12:00 PM
To: James Bottomley
Cc: Josh Boyer; Eric.Moore@lsi.com; Pravin Bathija; linux-scsi@vger.kernel.org; linuxppc-dev@ozlabs.org
Subject: Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.
 
On Thu, 2009-11-05 at 10:07 -0600, James Bottomley wrote:

> > > 	ioc->memmap = mem;
> > >-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
> > >-	    ioc->name, mem, mem_phys));
> > >+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
> > >+	    ioc->name, mem, (u64)mem_phys));
> > >
> > > 	ioc->mem_phys = mem_phys;
> > > 	ioc->chip = (SYSIF_REGS __iomem *)mem;
> > >
> > > 	/* Save Port IO values in case we need to do downloadboot */
> > >-	ioc->pio_mem_phys = port;
> > >+	port = ioremap(port_phys, psize);
> > >+	if (port == NULL) {
> > >+		printk(MYIOC_s_ERR_FMT " : ERROR - Unable to map adapter"
> > >+			" port !\n", ioc->name);
> > >+		return -EINVAL;
> 
> So this looks problematic on a few platforms ... what happens to
> platforms that have no IO space?  They automatically fail here and it
> looks like the adapter never attaches.

> Yup, that part of the patch looks wrong.

> However, a mechanical replacement of unsigned long's with
> resource_size_t to hold physical addresses should be fine despite the
> lack of feedback from LSI.

> Pravin, that ioremap definitely seems like it has nothing to do there,
> port IO is already remapped for you by the core PCI code and should work
> "as is". Please respin without that change.

> Cheers,
>Ben.

Thanks for the input. Will make the suggested changes and re-submit the patch.

Regards,
Pravin




[-- Attachment #2: Type: text/html, Size: 2991 bytes --]

RE: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Desai, Kashyap]

-----Original Message-----
From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel.org] On Behalf Of Benjamin Herrenschmidt
Sent: Friday, November 06, 2009 1:30 AM
To: James Bottomley
Cc: Josh Boyer; Moore, Eric; pbathija@amcc.com; linux-scsi@vger.kernel.org; linuxppc-dev@ozlabs.org
Subject: Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

On Thu, 2009-11-05 at 10:07 -0600, James Bottomley wrote:

> > > 	ioc->memmap = mem;
> > >-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
> > >-	    ioc->name, mem, mem_phys));
> > >+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
> > >+	    ioc->name, mem, (u64)mem_phys));
> > >
> > > 	ioc->mem_phys = mem_phys;
> > > 	ioc->chip = (SYSIF_REGS __iomem *)mem;
> > >
> > > 	/* Save Port IO values in case we need to do downloadboot */
> > >-	ioc->pio_mem_phys = port;
> > >+	port = ioremap(port_phys, psize);
> > >+	if (port == NULL) {
> > >+		printk(MYIOC_s_ERR_FMT " : ERROR - Unable to map adapter"
> > >+			" port !\n", ioc->name);
> > >+		return -EINVAL;
> 
> So this looks problematic on a few platforms ... what happens to
> platforms that have no IO space?  They automatically fail here and it
> looks like the adapter never attaches.

Yup, that part of the patch looks wrong.

However, a mechanical replacement of unsigned long's with
resource_size_t to hold physical addresses should be fine despite the
lack of feedback from LSI.

--> I was thinking this was actual fix for the issue. Use of resource_size_t is understood. Why submitter has added extra ioremap code in this patch? Because of ioremap code only this patch was on hold before going for ACK.
Pravin, Any specific reason to add above code?

Pravin, that ioremap definitely seems like it has nothing to do there,
port IO is already remapped for you by the core PCI code and should work
"as is". Please respin without that change.

Cheers,
Ben.


--
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] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Desai, Kashyap]

-----Original Message-----
From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-owner@vger.kernel=
.org] On Behalf Of Benjamin Herrenschmidt
Sent: Friday, November 06, 2009 1:30 AM
To: James Bottomley
Cc: Josh Boyer; Moore, Eric; pbathija@amcc.com; linux-scsi@vger.kernel.org;=
 linuxppc-dev@ozlabs.org
Subject: Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit re=
sources.

On Thu, 2009-11-05 at 10:07 -0600, James Bottomley wrote:

> > > 	ioc->memmap =3D mem;
> > >-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem =3D %p, mem_phys =3D %=
lx\n",
> > >-	    ioc->name, mem, mem_phys));
> > >+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem =3D %p, mem_phys =3D %=
llx\n",
> > >+	    ioc->name, mem, (u64)mem_phys));
> > >
> > > 	ioc->mem_phys =3D mem_phys;
> > > 	ioc->chip =3D (SYSIF_REGS __iomem *)mem;
> > >
> > > 	/* Save Port IO values in case we need to do downloadboot */
> > >-	ioc->pio_mem_phys =3D port;
> > >+	port =3D ioremap(port_phys, psize);
> > >+	if (port =3D=3D NULL) {
> > >+		printk(MYIOC_s_ERR_FMT " : ERROR - Unable to map adapter"
> > >+			" port !\n", ioc->name);
> > >+		return -EINVAL;
>=20
> So this looks problematic on a few platforms ... what happens to
> platforms that have no IO space?  They automatically fail here and it
> looks like the adapter never attaches.

Yup, that part of the patch looks wrong.

However, a mechanical replacement of unsigned long's with
resource_size_t to hold physical addresses should be fine despite the
lack of feedback from LSI.

--> I was thinking this was actual fix for the issue. Use of resource_size_=
t is understood. Why submitter has added extra ioremap code in this patch? =
Because of ioremap code only this patch was on hold before going for ACK.
Pravin, Any specific reason to add above code?

Pravin, that ioremap definitely seems like it has nothing to do there,
port IO is already remapped for you by the core PCI code and should work
"as is". Please respin without that change.

Cheers,
Ben.

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Benjamin Herrenschmidt]

On Thu, 2009-11-05 at 10:07 -0600, James Bottomley wrote:

> > > 	ioc->memmap = mem;
> > >-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
> > >-	    ioc->name, mem, mem_phys));
> > >+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
> > >+	    ioc->name, mem, (u64)mem_phys));
> > >
> > > 	ioc->mem_phys = mem_phys;
> > > 	ioc->chip = (SYSIF_REGS __iomem *)mem;
> > >
> > > 	/* Save Port IO values in case we need to do downloadboot */
> > >-	ioc->pio_mem_phys = port;
> > >+	port = ioremap(port_phys, psize);
> > >+	if (port == NULL) {
> > >+		printk(MYIOC_s_ERR_FMT " : ERROR - Unable to map adapter"
> > >+			" port !\n", ioc->name);
> > >+		return -EINVAL;
> 
> So this looks problematic on a few platforms ... what happens to
> platforms that have no IO space?  They automatically fail here and it
> looks like the adapter never attaches.

Yup, that part of the patch looks wrong.

However, a mechanical replacement of unsigned long's with
resource_size_t to hold physical addresses should be fine despite the
lack of feedback from LSI.

Pravin, that ioremap definitely seems like it has nothing to do there,
port IO is already remapped for you by the core PCI code and should work
"as is". Please respin without that change.

Cheers,
Ben.

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Josh Boyer]

On Thu, Nov 05, 2009 at 10:07:06AM -0600, James Bottomley wrote:
>On Thu, 2009-11-05 at 08:43 -0500, Josh Boyer wrote:
>> On Tue, Sep 15, 2009 at 03:25:55PM -0700, pbathija@amcc.com wrote:
>> >From: Pravin Bathija <pbathija@amcc.com>
>> >
>> >Powerpc 44x uses 36 bit real address while the real address defined
>> >in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
>> >fails to initialize. This fix changes the data types representing the real
>> >address from unsigned long 32-bit types to "phys_addr_t" which is 64-bit. The
>> >driver has been tested, the disks get discovered correctly and can do IO. Also,
>> >replaced phys_addr_t with resource_size_t as suggested by Ben.
>> >
>> >Signed-off-by: Pravin Bathija <pbathija@amcc.com>
>> >Acked-by: Feng Kan <fkan@amcc.com>
>> >Acked-by: Prodyut Hazarika <phazarika@amcc.com>
>> >Acked-by: Loc Ho <lho@amcc.com>
>> >Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
>> >Acked-by: Victor Gallardo <vgallardo@amcc.com>
>> 
>> Is this patch included in the scsi tree at all?  I can't seem to find it in
>> linux-next and I know it's not in the powerpc tree.  Are there further changes
>> needed, or has it simply been missed?
>
>What was the feedback from LSI ... I haven't seen any here?

I've seen no feedback on it anywhere.  The original patch used both
phys_addr_t and resource_size_t and Ben suggested they just use resource_size_t
everywhere.  Pravin reposted (which is this current patch) and it has sat here
since then.

josh

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Josh Boyer]

On Thu, Nov 05, 2009 at 10:07:06AM -0600, James Bottomley wrote:
>On Thu, 2009-11-05 at 08:43 -0500, Josh Boyer wrote:
>> On Tue, Sep 15, 2009 at 03:25:55PM -0700, pbathija@amcc.com wrote:
>> >From: Pravin Bathija <pbathija@amcc.com>
>> >
>> >Powerpc 44x uses 36 bit real address while the real address defined
>> >in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
>> >fails to initialize. This fix changes the data types representing the real
>> >address from unsigned long 32-bit types to "phys_addr_t" which is 64-bit. The
>> >driver has been tested, the disks get discovered correctly and can do IO. Also,
>> >replaced phys_addr_t with resource_size_t as suggested by Ben.
>> >
>> >Signed-off-by: Pravin Bathija <pbathija@amcc.com>
>> >Acked-by: Feng Kan <fkan@amcc.com>
>> >Acked-by: Prodyut Hazarika <phazarika@amcc.com>
>> >Acked-by: Loc Ho <lho@amcc.com>
>> >Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
>> >Acked-by: Victor Gallardo <vgallardo@amcc.com>
>> 
>> Is this patch included in the scsi tree at all?  I can't seem to find it in
>> linux-next and I know it's not in the powerpc tree.  Are there further changes
>> needed, or has it simply been missed?
>
>What was the feedback from LSI ... I haven't seen any here?

I've seen no feedback on it anywhere.  The original patch used both
phys_addr_t and resource_size_t and Ben suggested they just use resource_size_t
everywhere.  Pravin reposted (which is this current patch) and it has sat here
since then.

josh

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[James Bottomley]

On Thu, 2009-11-05 at 08:43 -0500, Josh Boyer wrote:
> On Tue, Sep 15, 2009 at 03:25:55PM -0700, pbathija@amcc.com wrote:
> >From: Pravin Bathija <pbathija@amcc.com>
> >
> >Powerpc 44x uses 36 bit real address while the real address defined
> >in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
> >fails to initialize. This fix changes the data types representing the real
> >address from unsigned long 32-bit types to "phys_addr_t" which is 64-bit. The
> >driver has been tested, the disks get discovered correctly and can do IO. Also,
> >replaced phys_addr_t with resource_size_t as suggested by Ben.
> >
> >Signed-off-by: Pravin Bathija <pbathija@amcc.com>
> >Acked-by: Feng Kan <fkan@amcc.com>
> >Acked-by: Prodyut Hazarika <phazarika@amcc.com>
> >Acked-by: Loc Ho <lho@amcc.com>
> >Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
> >Acked-by: Victor Gallardo <vgallardo@amcc.com>
> 
> Is this patch included in the scsi tree at all?  I can't seem to find it in
> linux-next and I know it's not in the powerpc tree.  Are there further changes
> needed, or has it simply been missed?

What was the feedback from LSI ... I haven't seen any here?

> josh
> 
> >
> >---
> > drivers/message/fusion/mptbase.c |   34 +++++++++++++++++++++++++---------
> > drivers/message/fusion/mptbase.h |    5 +++--
> > 2 files changed, 28 insertions(+), 11 deletions(-)
> >
> >diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> >index 5d496a9..e296f2e 100644
> >--- a/drivers/message/fusion/mptbase.c
> >+++ b/drivers/message/fusion/mptbase.c
> >@@ -1510,11 +1510,12 @@ static int
> > mpt_mapresources(MPT_ADAPTER *ioc)
> > {
> > 	u8		__iomem *mem;
> >+	u8		__iomem *port;
> > 	int		 ii;
> >-	unsigned long	 mem_phys;
> >-	unsigned long	 port;
> >-	u32		 msize;
> >-	u32		 psize;
> >+	resource_size_t	 mem_phys;
> >+	resource_size_t	 port_phys;
> >+	resource_size_t	 msize;
> >+	resource_size_t	 psize;
> > 	u8		 revision;
> > 	int		 r = -ENODEV;
> > 	struct pci_dev *pdev;
> >@@ -1552,13 +1553,13 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> > 	}
> >
> > 	mem_phys = msize = 0;
> >-	port = psize = 0;
> >+	port_phys = psize = 0;
> > 	for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) {
> > 		if (pci_resource_flags(pdev, ii) & PCI_BASE_ADDRESS_SPACE_IO) {
> > 			if (psize)
> > 				continue;
> > 			/* Get I/O space! */
> >-			port = pci_resource_start(pdev, ii);
> >+			port_phys = pci_resource_start(pdev, ii);
> > 			psize = pci_resource_len(pdev, ii);
> > 		} else {
> > 			if (msize)
> >@@ -1580,14 +1581,23 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> > 		return -EINVAL;
> > 	}
> > 	ioc->memmap = mem;
> >-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
> >-	    ioc->name, mem, mem_phys));
> >+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
> >+	    ioc->name, mem, (u64)mem_phys));
> >
> > 	ioc->mem_phys = mem_phys;
> > 	ioc->chip = (SYSIF_REGS __iomem *)mem;
> >
> > 	/* Save Port IO values in case we need to do downloadboot */
> >-	ioc->pio_mem_phys = port;
> >+	port = ioremap(port_phys, psize);
> >+	if (port == NULL) {
> >+		printk(MYIOC_s_ERR_FMT " : ERROR - Unable to map adapter"
> >+			" port !\n", ioc->name);
> >+		return -EINVAL;

So this looks problematic on a few platforms ... what happens to
platforms that have no IO space?  They automatically fail here and it
looks like the adapter never attaches.

James

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[James Bottomley]

On Thu, 2009-11-05 at 08:43 -0500, Josh Boyer wrote:
> On Tue, Sep 15, 2009 at 03:25:55PM -0700, pbathija@amcc.com wrote:
> >From: Pravin Bathija <pbathija@amcc.com>
> >
> >Powerpc 44x uses 36 bit real address while the real address defined
> >in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
> >fails to initialize. This fix changes the data types representing the real
> >address from unsigned long 32-bit types to "phys_addr_t" which is 64-bit. The
> >driver has been tested, the disks get discovered correctly and can do IO. Also,
> >replaced phys_addr_t with resource_size_t as suggested by Ben.
> >
> >Signed-off-by: Pravin Bathija <pbathija@amcc.com>
> >Acked-by: Feng Kan <fkan@amcc.com>
> >Acked-by: Prodyut Hazarika <phazarika@amcc.com>
> >Acked-by: Loc Ho <lho@amcc.com>
> >Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
> >Acked-by: Victor Gallardo <vgallardo@amcc.com>
> 
> Is this patch included in the scsi tree at all?  I can't seem to find it in
> linux-next and I know it's not in the powerpc tree.  Are there further changes
> needed, or has it simply been missed?

What was the feedback from LSI ... I haven't seen any here?

> josh
> 
> >
> >---
> > drivers/message/fusion/mptbase.c |   34 +++++++++++++++++++++++++---------
> > drivers/message/fusion/mptbase.h |    5 +++--
> > 2 files changed, 28 insertions(+), 11 deletions(-)
> >
> >diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> >index 5d496a9..e296f2e 100644
> >--- a/drivers/message/fusion/mptbase.c
> >+++ b/drivers/message/fusion/mptbase.c
> >@@ -1510,11 +1510,12 @@ static int
> > mpt_mapresources(MPT_ADAPTER *ioc)
> > {
> > 	u8		__iomem *mem;
> >+	u8		__iomem *port;
> > 	int		 ii;
> >-	unsigned long	 mem_phys;
> >-	unsigned long	 port;
> >-	u32		 msize;
> >-	u32		 psize;
> >+	resource_size_t	 mem_phys;
> >+	resource_size_t	 port_phys;
> >+	resource_size_t	 msize;
> >+	resource_size_t	 psize;
> > 	u8		 revision;
> > 	int		 r = -ENODEV;
> > 	struct pci_dev *pdev;
> >@@ -1552,13 +1553,13 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> > 	}
> >
> > 	mem_phys = msize = 0;
> >-	port = psize = 0;
> >+	port_phys = psize = 0;
> > 	for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) {
> > 		if (pci_resource_flags(pdev, ii) & PCI_BASE_ADDRESS_SPACE_IO) {
> > 			if (psize)
> > 				continue;
> > 			/* Get I/O space! */
> >-			port = pci_resource_start(pdev, ii);
> >+			port_phys = pci_resource_start(pdev, ii);
> > 			psize = pci_resource_len(pdev, ii);
> > 		} else {
> > 			if (msize)
> >@@ -1580,14 +1581,23 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> > 		return -EINVAL;
> > 	}
> > 	ioc->memmap = mem;
> >-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
> >-	    ioc->name, mem, mem_phys));
> >+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
> >+	    ioc->name, mem, (u64)mem_phys));
> >
> > 	ioc->mem_phys = mem_phys;
> > 	ioc->chip = (SYSIF_REGS __iomem *)mem;
> >
> > 	/* Save Port IO values in case we need to do downloadboot */
> >-	ioc->pio_mem_phys = port;
> >+	port = ioremap(port_phys, psize);
> >+	if (port == NULL) {
> >+		printk(MYIOC_s_ERR_FMT " : ERROR - Unable to map adapter"
> >+			" port !\n", ioc->name);
> >+		return -EINVAL;

So this looks problematic on a few platforms ... what happens to
platforms that have no IO space?  They automatically fail here and it
looks like the adapter never attaches.

James

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Josh Boyer]

On Tue, Sep 15, 2009 at 03:25:55PM -0700, pbathija@amcc.com wrote:
>From: Pravin Bathija <pbathija@amcc.com>
>
>Powerpc 44x uses 36 bit real address while the real address defined
>in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
>fails to initialize. This fix changes the data types representing the real
>address from unsigned long 32-bit types to "phys_addr_t" which is 64-bit. The
>driver has been tested, the disks get discovered correctly and can do IO. Also,
>replaced phys_addr_t with resource_size_t as suggested by Ben.
>
>Signed-off-by: Pravin Bathija <pbathija@amcc.com>
>Acked-by: Feng Kan <fkan@amcc.com>
>Acked-by: Prodyut Hazarika <phazarika@amcc.com>
>Acked-by: Loc Ho <lho@amcc.com>
>Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
>Acked-by: Victor Gallardo <vgallardo@amcc.com>

Is this patch included in the scsi tree at all?  I can't seem to find it in
linux-next and I know it's not in the powerpc tree.  Are there further changes
needed, or has it simply been missed?

josh

>
>---
> drivers/message/fusion/mptbase.c |   34 +++++++++++++++++++++++++---------
> drivers/message/fusion/mptbase.h |    5 +++--
> 2 files changed, 28 insertions(+), 11 deletions(-)
>
>diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
>index 5d496a9..e296f2e 100644
>--- a/drivers/message/fusion/mptbase.c
>+++ b/drivers/message/fusion/mptbase.c
>@@ -1510,11 +1510,12 @@ static int
> mpt_mapresources(MPT_ADAPTER *ioc)
> {
> 	u8		__iomem *mem;
>+	u8		__iomem *port;
> 	int		 ii;
>-	unsigned long	 mem_phys;
>-	unsigned long	 port;
>-	u32		 msize;
>-	u32		 psize;
>+	resource_size_t	 mem_phys;
>+	resource_size_t	 port_phys;
>+	resource_size_t	 msize;
>+	resource_size_t	 psize;
> 	u8		 revision;
> 	int		 r = -ENODEV;
> 	struct pci_dev *pdev;
>@@ -1552,13 +1553,13 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> 	}
>
> 	mem_phys = msize = 0;
>-	port = psize = 0;
>+	port_phys = psize = 0;
> 	for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) {
> 		if (pci_resource_flags(pdev, ii) & PCI_BASE_ADDRESS_SPACE_IO) {
> 			if (psize)
> 				continue;
> 			/* Get I/O space! */
>-			port = pci_resource_start(pdev, ii);
>+			port_phys = pci_resource_start(pdev, ii);
> 			psize = pci_resource_len(pdev, ii);
> 		} else {
> 			if (msize)
>@@ -1580,14 +1581,23 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> 		return -EINVAL;
> 	}
> 	ioc->memmap = mem;
>-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
>-	    ioc->name, mem, mem_phys));
>+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
>+	    ioc->name, mem, (u64)mem_phys));
>
> 	ioc->mem_phys = mem_phys;
> 	ioc->chip = (SYSIF_REGS __iomem *)mem;
>
> 	/* Save Port IO values in case we need to do downloadboot */
>-	ioc->pio_mem_phys = port;
>+	port = ioremap(port_phys, psize);
>+	if (port == NULL) {
>+		printk(MYIOC_s_ERR_FMT " : ERROR - Unable to map adapter"
>+			" port !\n", ioc->name);
>+		return -EINVAL;
>+	}
>+	ioc->portmap = port;
>+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "port=%p, port_phys=%llx\n",
>+			ioc->name, port, (u64)port_phys));
>+	ioc->pio_mem_phys = port_phys;
> 	ioc->pio_chip = (SYSIF_REGS __iomem *)port;
>
> 	return 0;
>@@ -1822,6 +1832,7 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
> 		if (ioc->alt_ioc)
> 			ioc->alt_ioc->alt_ioc = NULL;
> 		iounmap(ioc->memmap);
>+		iounmap(ioc->portmap);
> 		if (r != -5)
> 			pci_release_selected_regions(pdev, ioc->bars);
>
>@@ -2583,6 +2594,11 @@ mpt_adapter_dispose(MPT_ADAPTER *ioc)
> 		ioc->memmap = NULL;
> 	}
>
>+	if (ioc->portmap != NULL) {
>+		iounmap(ioc->portmap);
>+		ioc->portmap = NULL;
>+	}
>+
> 	pci_disable_device(ioc->pcidev);
> 	pci_release_selected_regions(ioc->pcidev, ioc->bars);
>
>diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
>index b3e981d..7091f13 100644
>--- a/drivers/message/fusion/mptbase.h
>+++ b/drivers/message/fusion/mptbase.h
>@@ -584,8 +584,8 @@ typedef struct _MPT_ADAPTER
> 	SYSIF_REGS __iomem	*chip;		/* == c8817000 (mmap) */
> 	SYSIF_REGS __iomem	*pio_chip;	/* Programmed IO (downloadboot) */
> 	u8			 bus_type;
>-	u32			 mem_phys;	/* == f4020000 (mmap) */
>-	u32			 pio_mem_phys;	/* Programmed IO (downloadboot) */
>+	resource_size_t		 mem_phys;	/* == f4020000 (mmap) */
>+	resource_size_t		 pio_mem_phys;	/* Programmed IO (downloadboot) */
> 	int			 mem_size;	/* mmap memory size */
> 	int			 number_of_buses;
> 	int			 devices_per_bus;
>@@ -635,6 +635,7 @@ typedef struct _MPT_ADAPTER
> 	int			bars;		/* bitmask of BAR's that must be configured */
> 	int			msi_enable;
> 	u8			__iomem *memmap;	/* mmap address */
>+	u8			__iomem *portmap;	/* mmap port address */
> 	struct Scsi_Host	*sh;		/* Scsi Host pointer */
> 	SpiCfgData		spi_data;	/* Scsi config. data */
> 	RaidCfgData		raid_data;	/* Raid config. data */
>-- 
>1.5.5
>
>_______________________________________________
>Linuxppc-dev mailing list
>Linuxppc-dev@lists.ozlabs.org
>https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[Josh Boyer]

On Tue, Sep 15, 2009 at 03:25:55PM -0700, pbathija@amcc.com wrote:
>From: Pravin Bathija <pbathija@amcc.com>
>
>Powerpc 44x uses 36 bit real address while the real address defined
>in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
>fails to initialize. This fix changes the data types representing the real
>address from unsigned long 32-bit types to "phys_addr_t" which is 64-bit. The
>driver has been tested, the disks get discovered correctly and can do IO. Also,
>replaced phys_addr_t with resource_size_t as suggested by Ben.
>
>Signed-off-by: Pravin Bathija <pbathija@amcc.com>
>Acked-by: Feng Kan <fkan@amcc.com>
>Acked-by: Prodyut Hazarika <phazarika@amcc.com>
>Acked-by: Loc Ho <lho@amcc.com>
>Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
>Acked-by: Victor Gallardo <vgallardo@amcc.com>

Is this patch included in the scsi tree at all?  I can't seem to find it in
linux-next and I know it's not in the powerpc tree.  Are there further changes
needed, or has it simply been missed?

josh

>
>---
> drivers/message/fusion/mptbase.c |   34 +++++++++++++++++++++++++---------
> drivers/message/fusion/mptbase.h |    5 +++--
> 2 files changed, 28 insertions(+), 11 deletions(-)
>
>diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
>index 5d496a9..e296f2e 100644
>--- a/drivers/message/fusion/mptbase.c
>+++ b/drivers/message/fusion/mptbase.c
>@@ -1510,11 +1510,12 @@ static int
> mpt_mapresources(MPT_ADAPTER *ioc)
> {
> 	u8		__iomem *mem;
>+	u8		__iomem *port;
> 	int		 ii;
>-	unsigned long	 mem_phys;
>-	unsigned long	 port;
>-	u32		 msize;
>-	u32		 psize;
>+	resource_size_t	 mem_phys;
>+	resource_size_t	 port_phys;
>+	resource_size_t	 msize;
>+	resource_size_t	 psize;
> 	u8		 revision;
> 	int		 r = -ENODEV;
> 	struct pci_dev *pdev;
>@@ -1552,13 +1553,13 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> 	}
>
> 	mem_phys = msize = 0;
>-	port = psize = 0;
>+	port_phys = psize = 0;
> 	for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) {
> 		if (pci_resource_flags(pdev, ii) & PCI_BASE_ADDRESS_SPACE_IO) {
> 			if (psize)
> 				continue;
> 			/* Get I/O space! */
>-			port = pci_resource_start(pdev, ii);
>+			port_phys = pci_resource_start(pdev, ii);
> 			psize = pci_resource_len(pdev, ii);
> 		} else {
> 			if (msize)
>@@ -1580,14 +1581,23 @@ mpt_mapresources(MPT_ADAPTER *ioc)
> 		return -EINVAL;
> 	}
> 	ioc->memmap = mem;
>-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
>-	    ioc->name, mem, mem_phys));
>+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
>+	    ioc->name, mem, (u64)mem_phys));
>
> 	ioc->mem_phys = mem_phys;
> 	ioc->chip = (SYSIF_REGS __iomem *)mem;
>
> 	/* Save Port IO values in case we need to do downloadboot */
>-	ioc->pio_mem_phys = port;
>+	port = ioremap(port_phys, psize);
>+	if (port == NULL) {
>+		printk(MYIOC_s_ERR_FMT " : ERROR - Unable to map adapter"
>+			" port !\n", ioc->name);
>+		return -EINVAL;
>+	}
>+	ioc->portmap = port;
>+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "port=%p, port_phys=%llx\n",
>+			ioc->name, port, (u64)port_phys));
>+	ioc->pio_mem_phys = port_phys;
> 	ioc->pio_chip = (SYSIF_REGS __iomem *)port;
>
> 	return 0;
>@@ -1822,6 +1832,7 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
> 		if (ioc->alt_ioc)
> 			ioc->alt_ioc->alt_ioc = NULL;
> 		iounmap(ioc->memmap);
>+		iounmap(ioc->portmap);
> 		if (r != -5)
> 			pci_release_selected_regions(pdev, ioc->bars);
>
>@@ -2583,6 +2594,11 @@ mpt_adapter_dispose(MPT_ADAPTER *ioc)
> 		ioc->memmap = NULL;
> 	}
>
>+	if (ioc->portmap != NULL) {
>+		iounmap(ioc->portmap);
>+		ioc->portmap = NULL;
>+	}
>+
> 	pci_disable_device(ioc->pcidev);
> 	pci_release_selected_regions(ioc->pcidev, ioc->bars);
>
>diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
>index b3e981d..7091f13 100644
>--- a/drivers/message/fusion/mptbase.h
>+++ b/drivers/message/fusion/mptbase.h
>@@ -584,8 +584,8 @@ typedef struct _MPT_ADAPTER
> 	SYSIF_REGS __iomem	*chip;		/* == c8817000 (mmap) */
> 	SYSIF_REGS __iomem	*pio_chip;	/* Programmed IO (downloadboot) */
> 	u8			 bus_type;
>-	u32			 mem_phys;	/* == f4020000 (mmap) */
>-	u32			 pio_mem_phys;	/* Programmed IO (downloadboot) */
>+	resource_size_t		 mem_phys;	/* == f4020000 (mmap) */
>+	resource_size_t		 pio_mem_phys;	/* Programmed IO (downloadboot) */
> 	int			 mem_size;	/* mmap memory size */
> 	int			 number_of_buses;
> 	int			 devices_per_bus;
>@@ -635,6 +635,7 @@ typedef struct _MPT_ADAPTER
> 	int			bars;		/* bitmask of BAR's that must be configured */
> 	int			msi_enable;
> 	u8			__iomem *memmap;	/* mmap address */
>+	u8			__iomem *portmap;	/* mmap port address */
> 	struct Scsi_Host	*sh;		/* Scsi Host pointer */
> 	SpiCfgData		spi_data;	/* Scsi config. data */
> 	RaidCfgData		raid_data;	/* Raid config. data */
>-- 
>1.5.5
>
>_______________________________________________
>Linuxppc-dev mailing list
>Linuxppc-dev@lists.ozlabs.org
>https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] [SCSI] mpt fusion: Fix 32 bit platforms with 64 bit resources.

[pbathija]

From: Pravin Bathija <pbathija@amcc.com>

Powerpc 44x uses 36 bit real address while the real address defined
in MPT Fusion driver is of type 32 bit. This causes ioremap to fail and driver
fails to initialize. This fix changes the data types representing the real
address from unsigned long 32-bit types to "phys_addr_t" which is 64-bit. The
driver has been tested, the disks get discovered correctly and can do IO. Also,
replaced phys_addr_t with resource_size_t as suggested by Ben.

Signed-off-by: Pravin Bathija <pbathija@amcc.com>
Acked-by: Feng Kan <fkan@amcc.com>
Acked-by: Prodyut Hazarika <phazarika@amcc.com>
Acked-by: Loc Ho <lho@amcc.com>
Acked-by: Tirumala Reddy Marri <tmarri@amcc.com>
Acked-by: Victor Gallardo <vgallardo@amcc.com>

---
 drivers/message/fusion/mptbase.c |   34 +++++++++++++++++++++++++---------
 drivers/message/fusion/mptbase.h |    5 +++--
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 5d496a9..e296f2e 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1510,11 +1510,12 @@ static int
 mpt_mapresources(MPT_ADAPTER *ioc)
 {
 	u8		__iomem *mem;
+	u8		__iomem *port;
 	int		 ii;
-	unsigned long	 mem_phys;
-	unsigned long	 port;
-	u32		 msize;
-	u32		 psize;
+	resource_size_t	 mem_phys;
+	resource_size_t	 port_phys;
+	resource_size_t	 msize;
+	resource_size_t	 psize;
 	u8		 revision;
 	int		 r = -ENODEV;
 	struct pci_dev *pdev;
@@ -1552,13 +1553,13 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 	}
 
 	mem_phys = msize = 0;
-	port = psize = 0;
+	port_phys = psize = 0;
 	for (ii = 0; ii < DEVICE_COUNT_RESOURCE; ii++) {
 		if (pci_resource_flags(pdev, ii) & PCI_BASE_ADDRESS_SPACE_IO) {
 			if (psize)
 				continue;
 			/* Get I/O space! */
-			port = pci_resource_start(pdev, ii);
+			port_phys = pci_resource_start(pdev, ii);
 			psize = pci_resource_len(pdev, ii);
 		} else {
 			if (msize)
@@ -1580,14 +1581,23 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 		return -EINVAL;
 	}
 	ioc->memmap = mem;
-	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %lx\n",
-	    ioc->name, mem, mem_phys));
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "mem = %p, mem_phys = %llx\n",
+	    ioc->name, mem, (u64)mem_phys));
 
 	ioc->mem_phys = mem_phys;
 	ioc->chip = (SYSIF_REGS __iomem *)mem;
 
 	/* Save Port IO values in case we need to do downloadboot */
-	ioc->pio_mem_phys = port;
+	port = ioremap(port_phys, psize);
+	if (port == NULL) {
+		printk(MYIOC_s_ERR_FMT " : ERROR - Unable to map adapter"
+			" port !\n", ioc->name);
+		return -EINVAL;
+	}
+	ioc->portmap = port;
+	dinitprintk(ioc, printk(MYIOC_s_INFO_FMT "port=%p, port_phys=%llx\n",
+			ioc->name, port, (u64)port_phys));
+	ioc->pio_mem_phys = port_phys;
 	ioc->pio_chip = (SYSIF_REGS __iomem *)port;
 
 	return 0;
@@ -1822,6 +1832,7 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 		if (ioc->alt_ioc)
 			ioc->alt_ioc->alt_ioc = NULL;
 		iounmap(ioc->memmap);
+		iounmap(ioc->portmap);
 		if (r != -5)
 			pci_release_selected_regions(pdev, ioc->bars);
 
@@ -2583,6 +2594,11 @@ mpt_adapter_dispose(MPT_ADAPTER *ioc)
 		ioc->memmap = NULL;
 	}
 
+	if (ioc->portmap != NULL) {
+		iounmap(ioc->portmap);
+		ioc->portmap = NULL;
+	}
+
 	pci_disable_device(ioc->pcidev);
 	pci_release_selected_regions(ioc->pcidev, ioc->bars);
 
diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index b3e981d..7091f13 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -584,8 +584,8 @@ typedef struct _MPT_ADAPTER
 	SYSIF_REGS __iomem	*chip;		/* == c8817000 (mmap) */
 	SYSIF_REGS __iomem	*pio_chip;	/* Programmed IO (downloadboot) */
 	u8			 bus_type;
-	u32			 mem_phys;	/* == f4020000 (mmap) */
-	u32			 pio_mem_phys;	/* Programmed IO (downloadboot) */
+	resource_size_t		 mem_phys;	/* == f4020000 (mmap) */
+	resource_size_t		 pio_mem_phys;	/* Programmed IO (downloadboot) */
 	int			 mem_size;	/* mmap memory size */
 	int			 number_of_buses;
 	int			 devices_per_bus;
@@ -635,6 +635,7 @@ typedef struct _MPT_ADAPTER
 	int			bars;		/* bitmask of BAR's that must be configured */
 	int			msi_enable;
 	u8			__iomem *memmap;	/* mmap address */
+	u8			__iomem *portmap;	/* mmap port address */
 	struct Scsi_Host	*sh;		/* Scsi Host pointer */
 	SpiCfgData		spi_data;	/* Scsi config. data */
 	RaidCfgData		raid_data;	/* Raid config. data */
-- 
1.5.5