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

2009-12-10 Thread 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 pbath...@amcc.com
 Acked-by: Feng Kan f...@amcc.com
 Acked-by: Fushen Chen fc...@amcc.com
 Acked-by: Loc Ho l...@amcc.com
 Acked-by: Tirumala Reddy Marri tma...@amcc.com
 Acked-by: Victor Gallardo vgalla...@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 longmem_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;


___
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.

2009-12-10 Thread Anatolij Gustschin
On Thu, 10 Dec 2009 09:43:38 -0600
James Bottomley james.bottom...@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 pbath...@amcc.com
  Acked-by: Feng Kan f...@amcc.com
  Acked-by: Fushen Chen fc...@amcc.com
  Acked-by: Loc Ho l...@amcc.com
  Acked-by: Tirumala Reddy Marri tma...@amcc.com
  Acked-by: Victor Gallardo vgalla...@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 longmem_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;  /* == f402 (mmap) */
+   resource_size_t  mem_phys;  /* == f402 (mmap) */
u32  pio_mem_phys;  /* Programmed IO (downloadboot) 
*/
int  mem_size;  /* mmap memory size */
int  number_of_buses;
___
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.

2009-12-03 Thread Wolfgang Denk
Dear Pravin Bathija,

In message 1259805106-23636-1-git-send-email-pbath...@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 longmem_phys;
 + resource_size_t  mem_phys;
   unsigned longport;
   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 y...@emcraft.com
To: linux-s...@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 y...@emcraft.com
Signed-off-by: Ilya Yanok ya...@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 longmem_phys;
-   unsigned longport;
-   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);

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

2009-12-03 Thread Pravin Bathija
Hi Wolfgang,

 -Original Message-
 From: Wolfgang Denk [mailto:w...@denx.de]
 Sent: Thursday, December 03, 2009 12:56 AM
 To: Pravin Bathija; Benjamin Herrenschmidt; Desai, Kashyap
 Cc: linux-s...@vger.kernel.org; linuxppc-...@ozlabs.org;
 eric.mo...@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-pbath...@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 longmem_phys;
  +   resource_size_t  mem_phys;
  unsigned longport;
  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 y...@emcraft.com
 To: linux-s...@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 y...@emcraft.com
 Signed-off-by: Ilya Yanok ya...@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 longmem_phys;
 - unsigned longport;
 - 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

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

2009-12-03 Thread 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.


___
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.

2009-12-02 Thread 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 pbath...@amcc.com

Acked-by: Benjamin Herrenschmidt b...@kernel.crashing.org
---

James, this one should be good ;-)

 Acked-by: Feng Kan f...@amcc.com
 Acked-by: Fushen Chen fc...@amcc.com
 Acked-by: Loc Ho l...@amcc.com
 Acked-by: Tirumala Reddy Marri tma...@amcc.com
 Acked-by: Victor Gallardo vgalla...@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 longmem_phys;
 + resource_size_t  mem_phys;
   unsigned longport;
   u32  msize;
   u32  psize;


___
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.

2009-12-02 Thread Desai, Kashyap

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

- Kashyap

 -Original Message-
 From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
 ow...@vger.kernel.org] On Behalf Of Benjamin Herrenschmidt
 Sent: Thursday, December 03, 2009 8:30 AM
 To: Pravin Bathija
 Cc: linux-s...@vger.kernel.org; linuxppc-...@ozlabs.org;
 jwbo...@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 pbath...@amcc.com
 
 Acked-by: Benjamin Herrenschmidt b...@kernel.crashing.org
 ---
 
 James, this one should be good ;-)
 
  Acked-by: Feng Kan f...@amcc.com
  Acked-by: Fushen Chen fc...@amcc.com
  Acked-by: Loc Ho l...@amcc.com
  Acked-by: Tirumala Reddy Marri tma...@amcc.com
  Acked-by: Victor Gallardo vgalla...@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 longmem_phys;
  +   resource_size_t  mem_phys;
  unsigned longport;
  u32  msize;
  u32  psize;
 
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-scsi in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
___
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.

2009-11-17 Thread Benjamin Herrenschmidt
On Tue, 2009-11-17 at 16:16 -0800, pbath...@amcc.com wrote:
 From: Pravin Bathija pbath...@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.


___
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.

2009-11-05 Thread 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.


___
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.

2009-11-05 Thread James Bottomley
On Thu, 2009-11-05 at 08:43 -0500, Josh Boyer wrote:
 On Tue, Sep 15, 2009 at 03:25:55PM -0700, pbath...@amcc.com wrote:
 From: Pravin Bathija pbath...@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 pbath...@amcc.com
 Acked-by: Feng Kan f...@amcc.com
 Acked-by: Prodyut Hazarika phazar...@amcc.com
 Acked-by: Loc Ho l...@amcc.com
 Acked-by: Tirumala Reddy Marri tma...@amcc.com
 Acked-by: Victor Gallardo vgalla...@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 longmem_phys;
 -unsigned longport;
 -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


___
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.

2009-11-05 Thread Pravin Bathija


 -Original Message-
 From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org]
 Sent: Thursday, November 05, 2009 12:00 PM
 To: James Bottomley
 Cc: Josh Boyer; eric.mo...@lsi.com; Pravin Bathija; linux-
 s...@vger.kernel.org; linuxppc-...@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
___
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.

2009-11-05 Thread Desai, Kashyap


-Original Message-
From: linux-scsi-ow...@vger.kernel.org 
[mailto:linux-scsi-ow...@vger.kernel.org] On Behalf Of Benjamin Herrenschmidt
Sent: Friday, November 06, 2009 1:30 AM
To: James Bottomley
Cc: Josh Boyer; Moore, Eric; pbath...@amcc.com; linux-s...@vger.kernel.org; 
linuxppc-...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
___
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.

2009-11-05 Thread Pravin Bathija
-Original Message-
From: Benjamin Herrenschmidt [mailto:b...@kernel.crashing.org]
Sent: Thu 11/5/2009 12:00 PM
To: James Bottomley
Cc: Josh Boyer; eric.mo...@lsi.com; Pravin Bathija; linux-s...@vger.kernel.org; 
linuxppc-...@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



___
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

2009-09-15 Thread 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 longmem_phys;
 - unsigned longport;
 - 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.


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