All of lore.kernel.org
 help / color / mirror / Atom feed
* sata_sil problem / oops
@ 2005-06-02 10:03 Marcus Meissner
  2005-06-02 17:24 ` Daniela Engert
  0 siblings, 1 reply; 6+ messages in thread
From: Marcus Meissner @ 2005-06-02 10:03 UTC (permalink / raw)
  To: jgarzik, linux-ide, Jens Axboe

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

Hi folks,

We see a problem in the sata_sil driver. The code looks like this:

        cls=sil_get_device_cache_line(pdev);
        cls >>= 3;
        cls++;  /* cls = (line_size/8)+1 */
        writeb(cls, mmio_base + SIL_FIFO_R0);
        writeb(cls, mmio_base + SIL_FIFO_W0);
        writeb(cls, mmio_base + SIL_FIFO_R1);
        writeb(cls, mmio_base + SIL_FIFO_W2);

We have a device where mmio_base is only 0x200 byte long, so
the access to SIL_FIFO_W2 (offset 0x241) causes an Oops.

- Should it perhaps be W1 instead of W2?

- If not, does it need a range check?

sysfs PCI data:

 pci device: name = 0000:00:0a.0, bus_id = 0000:00:0a.0, bus = pci
    path = /devices/pci0000:00/0000:00:0a.0
    class = 0x10400
    vendor = 0x1095
    device = 0x3112
    subvendor = 0x1095
    subdevice = 0x6112
    irq = 5
    res[0] = 0xdc00 0xdc07 0x101
    res[1] = 0xd800 0xd803 0x101
    res[2] = 0xd400 0xd407 0x101
    res[3] = 0xd000 0xd003 0x101
    res[4] = 0xcc00 0xcc0f 0x101
    res[5] = 0xcffffe00 0xcfffffff 0x200
    res[6] = 0xcff00000 0xcff7ffff 0x7200
    config[64]

Ciao, Marcus

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sata_sil problem / oops
  2005-06-02 10:03 sata_sil problem / oops Marcus Meissner
@ 2005-06-02 17:24 ` Daniela Engert
  2005-06-02 19:23   ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: Daniela Engert @ 2005-06-02 17:24 UTC (permalink / raw)
  To: linux-ide@vger.kernel.org

On Thu, 2 Jun 2005 12:03:58 +0200, Marcus Meissner wrote:

>We see a problem in the sata_sil driver. The code looks like this:
>
>	 cls=sil_get_device_cache_line(pdev);
>	 cls >>= 3;
>	 cls++;  /* cls = (line_size/8)+1 */
>	 writeb(cls, mmio_base + SIL_FIFO_R0);
>	 writeb(cls, mmio_base + SIL_FIFO_W0);
>	 writeb(cls, mmio_base + SIL_FIFO_R1);
>	 writeb(cls, mmio_base + SIL_FIFO_W2);
>
>We have a device where mmio_base is only 0x200 byte long, so
>the access to SIL_FIFO_W2 (offset 0x241) causes an Oops.
>
>- Should it perhaps be W1 instead of W2?
>- If not, does it need a range check?

My OS/2 driver and the Windows driver write to the following locations:

SiI3112: BAR5+0x40, BAR5+0x41, BAR5+0x44, BAR5+0x45
SiI3114: BAR5+0x40, BAR5+0x41, BAR5+0x44, BAR5+0x45,
	 BAR5+0x240, BAR5+0x241, BAR5+0x244, BAR5+0x245

Ciao,
  Dani



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sata_sil problem / oops
  2005-06-02 17:24 ` Daniela Engert
@ 2005-06-02 19:23   ` Jens Axboe
  2005-06-02 19:30     ` Jens Axboe
  2005-06-02 19:50     ` Daniela Engert
  0 siblings, 2 replies; 6+ messages in thread
From: Jens Axboe @ 2005-06-02 19:23 UTC (permalink / raw)
  To: Daniela Engert; +Cc: linux-ide@vger.kernel.org

On Thu, Jun 02 2005, Daniela Engert wrote:
> On Thu, 2 Jun 2005 12:03:58 +0200, Marcus Meissner wrote:
> 
> >We see a problem in the sata_sil driver. The code looks like this:
> >
> >	 cls=sil_get_device_cache_line(pdev);
> >	 cls >>= 3;
> >	 cls++;  /* cls = (line_size/8)+1 */
> >	 writeb(cls, mmio_base + SIL_FIFO_R0);
> >	 writeb(cls, mmio_base + SIL_FIFO_W0);
> >	 writeb(cls, mmio_base + SIL_FIFO_R1);
> >	 writeb(cls, mmio_base + SIL_FIFO_W2);
> >
> >We have a device where mmio_base is only 0x200 byte long, so
> >the access to SIL_FIFO_W2 (offset 0x241) causes an Oops.
> >
> >- Should it perhaps be W1 instead of W2?
> >- If not, does it need a range check?
> 
> My OS/2 driver and the Windows driver write to the following locations:
> 
> SiI3112: BAR5+0x40, BAR5+0x41, BAR5+0x44, BAR5+0x45
> SiI3114: BAR5+0x40, BAR5+0x41, BAR5+0x44, BAR5+0x45,
> 	 BAR5+0x240, BAR5+0x241, BAR5+0x244, BAR5+0x245

Does the 3112 have two ports and the 3114 four?

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sata_sil problem / oops
  2005-06-02 19:23   ` Jens Axboe
@ 2005-06-02 19:30     ` Jens Axboe
  2005-06-02 21:45       ` Jeff Garzik
  2005-06-02 19:50     ` Daniela Engert
  1 sibling, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2005-06-02 19:30 UTC (permalink / raw)
  To: Daniela Engert; +Cc: linux-ide@vger.kernel.org, meissner, Jeff Garzik

On Thu, Jun 02 2005, Jens Axboe wrote:
> On Thu, Jun 02 2005, Daniela Engert wrote:
> > On Thu, 2 Jun 2005 12:03:58 +0200, Marcus Meissner wrote:
> > 
> > >We see a problem in the sata_sil driver. The code looks like this:
> > >
> > >	 cls=sil_get_device_cache_line(pdev);
> > >	 cls >>= 3;
> > >	 cls++;  /* cls = (line_size/8)+1 */
> > >	 writeb(cls, mmio_base + SIL_FIFO_R0);
> > >	 writeb(cls, mmio_base + SIL_FIFO_W0);
> > >	 writeb(cls, mmio_base + SIL_FIFO_R1);
> > >	 writeb(cls, mmio_base + SIL_FIFO_W2);
> > >
> > >We have a device where mmio_base is only 0x200 byte long, so
> > >the access to SIL_FIFO_W2 (offset 0x241) causes an Oops.
> > >
> > >- Should it perhaps be W1 instead of W2?
> > >- If not, does it need a range check?
> > 
> > My OS/2 driver and the Windows driver write to the following locations:
> > 
> > SiI3112: BAR5+0x40, BAR5+0x41, BAR5+0x44, BAR5+0x45
> > SiI3114: BAR5+0x40, BAR5+0x41, BAR5+0x44, BAR5+0x45,
> > 	 BAR5+0x240, BAR5+0x241, BAR5+0x244, BAR5+0x245
> 
> Does the 3112 have two ports and the 3114 four?

Seems that is so, from looking at the driver. If I were to guess at a
fix for this, it would be as follows.

diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -432,7 +432,13 @@ static int sil_init_one (struct pci_dev 
 		writeb(cls, mmio_base + SIL_FIFO_R0);
 		writeb(cls, mmio_base + SIL_FIFO_W0);
 		writeb(cls, mmio_base + SIL_FIFO_R1);
-		writeb(cls, mmio_base + SIL_FIFO_W2);
+		writeb(cls, mmio_base + SIL_FIFO_W1);
+		if (ent->driver_data == sil_3114) {
+			writeb(cls, mmio_base + SIL_FIFO_R2);
+			writeb(cls, mmio_base + SIL_FIFO_W2);
+			writeb(cls, mmio_base + SIL_FIFO_R3);
+			writeb(cls, mmio_base + SIL_FIFO_W3);
+		}
 	} else
 		printk(KERN_WARNING DRV_NAME "(%s): cache line size not set.  Driver may not function\n",
 			pci_name(pdev));

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sata_sil problem / oops
  2005-06-02 19:23   ` Jens Axboe
  2005-06-02 19:30     ` Jens Axboe
@ 2005-06-02 19:50     ` Daniela Engert
  1 sibling, 0 replies; 6+ messages in thread
From: Daniela Engert @ 2005-06-02 19:50 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-ide@vger.kernel.org

On Thu, 2 Jun 2005 21:23:29 +0200, Jens Axboe wrote:

>On Thu, Jun 02 2005, Daniela Engert wrote:

>> My OS/2 driver and the Windows driver write to the following locations:
>> 
>> SiI3112: BAR5+0x40, BAR5+0x41, BAR5+0x44, BAR5+0x45
>> SiI3114: BAR5+0x40, BAR5+0x41, BAR5+0x44, BAR5+0x45,
>> 	 BAR5+0x240, BAR5+0x241, BAR5+0x244, BAR5+0x245
>
>Does the 3112 have two ports and the 3114 four?

Exactly.

Ciao,
  Dani



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: sata_sil problem / oops
  2005-06-02 19:30     ` Jens Axboe
@ 2005-06-02 21:45       ` Jeff Garzik
  0 siblings, 0 replies; 6+ messages in thread
From: Jeff Garzik @ 2005-06-02 21:45 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Daniela Engert, linux-ide@vger.kernel.org, meissner

Jens Axboe wrote:
> Seems that is so, from looking at the driver. If I were to guess at a
> fix for this, it would be as follows.
> 
> diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
> --- a/drivers/scsi/sata_sil.c
> +++ b/drivers/scsi/sata_sil.c
> @@ -432,7 +432,13 @@ static int sil_init_one (struct pci_dev 
>  		writeb(cls, mmio_base + SIL_FIFO_R0);
>  		writeb(cls, mmio_base + SIL_FIFO_W0);
>  		writeb(cls, mmio_base + SIL_FIFO_R1);
> -		writeb(cls, mmio_base + SIL_FIFO_W2);
> +		writeb(cls, mmio_base + SIL_FIFO_W1);
> +		if (ent->driver_data == sil_3114) {
> +			writeb(cls, mmio_base + SIL_FIFO_R2);
> +			writeb(cls, mmio_base + SIL_FIFO_W2);
> +			writeb(cls, mmio_base + SIL_FIFO_R3);
> +			writeb(cls, mmio_base + SIL_FIFO_W3);
> +		}


This definitely looks like a needed fix to me.

Can someone confirm that this fixes the reported problem?

	Jeff



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2005-06-02 21:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-06-02 10:03 sata_sil problem / oops Marcus Meissner
2005-06-02 17:24 ` Daniela Engert
2005-06-02 19:23   ` Jens Axboe
2005-06-02 19:30     ` Jens Axboe
2005-06-02 21:45       ` Jeff Garzik
2005-06-02 19:50     ` Daniela Engert

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.