linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: 2.5.7 pre-UDMA PIIX bug
@ 2002-03-29 18:20 Mikael Pettersson
  0 siblings, 0 replies; 7+ messages in thread
From: Mikael Pettersson @ 2002-03-29 18:20 UTC (permalink / raw)
  To: vojtech; +Cc: linux-kernel, martin

On Fri, 29 Mar 2002 16:08:18 +0100, Vojtech Pavlik wrote:
>diff -urN linux-2.5.7/drivers/ide/piix.c linux-2.5.7-fixpiix/drivers/ide/piix.c
>--- linux-2.5.7/drivers/ide/piix.c	Fri Mar 29 16:01:51 2002
>+++ linux-2.5.7-fixpiix/drivers/ide/piix.c	Fri Mar 29 16:07:05 2002
...
>@@ -331,7 +331,7 @@
> 		umul = 2;
> 
> 	T = 1000000000 / piix_clock;
>-	UT = T / umul;
>+	UT = umul ? (T / umul) : 0;

That did indeed fix the problem. Thanks.

/Mikael

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

* Re: 2.5.7 pre-UDMA PIIX bug
  2002-03-29 15:37   ` Bill Davidsen
@ 2002-03-30 14:26     ` Martin Dalecki
  0 siblings, 0 replies; 7+ messages in thread
From: Martin Dalecki @ 2002-03-30 14:26 UTC (permalink / raw)
  To: Bill Davidsen; +Cc: Mikael Pettersson, vojtech, linux-kernel

Bill Davidsen wrote:
> On Fri, 29 Mar 2002, Martin Dalecki wrote:
> 
> 
>>>  333		T = 1000000000 / piix_clock;
>>>  334		UT = T / umul;
>>
>>I think that it should be just sufficient to add the
>>following test just in front of the offending calculartion.
>>
>>if (umul == 0)
>>   ++umul;
> 
> 
> - 334               UT = T / umul;
> + 334               UT = T / (umul || 1);

That is ceratily wrong. becouse umul || 1 == 1 independently
from umul.


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

* Re: 2.5.7 pre-UDMA PIIX bug
  2002-03-29 14:00 ` Martin Dalecki
  2002-03-29 15:09   ` Vojtech Pavlik
@ 2002-03-29 15:37   ` Bill Davidsen
  2002-03-30 14:26     ` Martin Dalecki
  1 sibling, 1 reply; 7+ messages in thread
From: Bill Davidsen @ 2002-03-29 15:37 UTC (permalink / raw)
  To: Martin Dalecki; +Cc: Mikael Pettersson, vojtech, linux-kernel

On Fri, 29 Mar 2002, Martin Dalecki wrote:

> >   333		T = 1000000000 / piix_clock;
> >   334		UT = T / umul;
> 
> I think that it should be just sufficient to add the
> following test just in front of the offending calculartion.
> 
> if (umul == 0)
>    ++umul;

- 334               UT = T / umul;
+ 334               UT = T / (umul || 1);

-- 
bill davidsen <davidsen@tmr.com>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.


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

* Re: 2.5.7 pre-UDMA PIIX bug
  2002-03-29 14:00 ` Martin Dalecki
@ 2002-03-29 15:09   ` Vojtech Pavlik
  2002-03-29 15:37   ` Bill Davidsen
  1 sibling, 0 replies; 7+ messages in thread
From: Vojtech Pavlik @ 2002-03-29 15:09 UTC (permalink / raw)
  To: Martin Dalecki; +Cc: Mikael Pettersson, linux-kernel

On Fri, Mar 29, 2002 at 03:00:24PM +0100, Martin Dalecki wrote:
> Mikael Pettersson wrote:
> > Vojtech's version of drivers/ide/piix.c which went into 2.5.7
> > oopses with a divide-by-zero exception when initialising older
> > pre-UDMA chips, like in the following 430HX chipset:
> > 
> > 00:00.0 Host bridge: Intel Corporation 430HX - 82439HX TXC [Triton II] (rev 03)
> > 00:07.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II] (rev 01)
> > 00:07.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
> > (PCI IDs 8086:1250, 8086:7000, and 8086:7010, respectively)
> > 
> > The error occurs in piix.c:piix_set_drive() line 334, shown below.
> > The 82371SB has PIIX_UDMA_NONE in the piix_ide_chips[] array,
> > so piix_config->flags & PIIX_UDMA is zero, which makes "umul" zero,
> > which causes the divide-by-zero on line 334.
> > 
> >   317	static int piix_set_drive(ide_drive_t *drive, unsigned char speed)
> >   318	{
> >   319		ide_drive_t *peer = HWIF(drive)->drives + (~drive->dn & 1);
> >   320		struct ata_timing t, p;
> >   321		int err, T, UT, umul;
> >   322	
> >   323		if (speed != XFER_PIO_SLOW && speed != drive->current_speed)
> >   324			if ((err = ide_config_drive_speed(drive, speed)))
> >   325				return err;
> >   326	
> >   327		umul =  min((speed > XFER_UDMA_4) ? 4 : ((speed > XFER_UDMA_2) ? 2 : 1),
> >   328			piix_config->flags & PIIX_UDMA);
> >   329	
> >   330		if (piix_config->flags & PIIX_VICTORY)
> >   331			umul = 2;
> >   332	
> >   333		T = 1000000000 / piix_clock;
> >   334		UT = T / umul;
> 
> I think that it should be just sufficient to add the
> following test just in front of the offending calculartion.
> 
> if (umul == 0)
>    ++umul;
> 
> Vojtech is this right?

Probably yes, the value of UT isn't used in that case, because we will
never try to set an UDMA mode on such a chipset. I've sent you a
slightly different patch, though.

-- 
Vojtech Pavlik
SuSE Labs

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

* Re: 2.5.7 pre-UDMA PIIX bug
  2002-03-29 12:39 Mikael Pettersson
  2002-03-29 14:00 ` Martin Dalecki
@ 2002-03-29 15:08 ` Vojtech Pavlik
  1 sibling, 0 replies; 7+ messages in thread
From: Vojtech Pavlik @ 2002-03-29 15:08 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: linux-kernel, martin

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

On Fri, Mar 29, 2002 at 01:39:56PM +0100, Mikael Pettersson wrote:
> Vojtech's version of drivers/ide/piix.c which went into 2.5.7
> oopses with a divide-by-zero exception when initialising older
> pre-UDMA chips, like in the following 430HX chipset:
> 
> 00:00.0 Host bridge: Intel Corporation 430HX - 82439HX TXC [Triton II] (rev 03)
> 00:07.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II] (rev 01)
> 00:07.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
> (PCI IDs 8086:1250, 8086:7000, and 8086:7010, respectively)
> 
> The error occurs in piix.c:piix_set_drive() line 334, shown below.
> The 82371SB has PIIX_UDMA_NONE in the piix_ide_chips[] array,
> so piix_config->flags & PIIX_UDMA is zero, which makes "umul" zero,
> which causes the divide-by-zero on line 334.
> 
>   317	static int piix_set_drive(ide_drive_t *drive, unsigned char speed)
>   318	{
>   319		ide_drive_t *peer = HWIF(drive)->drives + (~drive->dn & 1);
>   320		struct ata_timing t, p;
>   321		int err, T, UT, umul;
>   322	
>   323		if (speed != XFER_PIO_SLOW && speed != drive->current_speed)
>   324			if ((err = ide_config_drive_speed(drive, speed)))
>   325				return err;
>   326	
>   327		umul =  min((speed > XFER_UDMA_4) ? 4 : ((speed > XFER_UDMA_2) ? 2 : 1),
>   328			piix_config->flags & PIIX_UDMA);
>   329	
>   330		if (piix_config->flags & PIIX_VICTORY)
>   331			umul = 2;
>   332	
>   333		T = 1000000000 / piix_clock;
>   334		UT = T / umul;

Thanks for the bug report! The attached patch should fix that.

Martin, please apply this patch.

-- 
Vojtech Pavlik
SuSE Labs

[-- Attachment #2: fixpiix.diff --]
[-- Type: text/plain, Size: 897 bytes --]

diff -urN linux-2.5.7/drivers/ide/piix.c linux-2.5.7-fixpiix/drivers/ide/piix.c
--- linux-2.5.7/drivers/ide/piix.c	Fri Mar 29 16:01:51 2002
+++ linux-2.5.7-fixpiix/drivers/ide/piix.c	Fri Mar 29 16:07:05 2002
@@ -1,5 +1,5 @@
 /*
- * $Id: piix.c,v 1.2 2002/03/13 22:50:43 vojtech Exp $
+ * $Id: piix.c,v 1.3 2002/03/29 16:06:06 vojtech Exp $
  *
  *  Copyright (c) 2000-2002 Vojtech Pavlik
  *
@@ -128,7 +128,7 @@
 
 	piix_print("----------PIIX BusMastering IDE Configuration---------------");
 
-	piix_print("Driver Version:                     1.2");
+	piix_print("Driver Version:                     1.3");
 	piix_print("South Bridge:                       %s", bmide_dev->name);
 
 	pci_read_config_byte(dev, PCI_REVISION_ID, &t);
@@ -331,7 +331,7 @@
 		umul = 2;
 
 	T = 1000000000 / piix_clock;
-	UT = T / umul;
+	UT = umul ? (T / umul) : 0;
 
 	ata_timing_compute(drive, speed, &t, T, UT);
 

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

* Re: 2.5.7 pre-UDMA PIIX bug
  2002-03-29 12:39 Mikael Pettersson
@ 2002-03-29 14:00 ` Martin Dalecki
  2002-03-29 15:09   ` Vojtech Pavlik
  2002-03-29 15:37   ` Bill Davidsen
  2002-03-29 15:08 ` Vojtech Pavlik
  1 sibling, 2 replies; 7+ messages in thread
From: Martin Dalecki @ 2002-03-29 14:00 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: vojtech, linux-kernel

Mikael Pettersson wrote:
> Vojtech's version of drivers/ide/piix.c which went into 2.5.7
> oopses with a divide-by-zero exception when initialising older
> pre-UDMA chips, like in the following 430HX chipset:
> 
> 00:00.0 Host bridge: Intel Corporation 430HX - 82439HX TXC [Triton II] (rev 03)
> 00:07.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II] (rev 01)
> 00:07.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
> (PCI IDs 8086:1250, 8086:7000, and 8086:7010, respectively)
> 
> The error occurs in piix.c:piix_set_drive() line 334, shown below.
> The 82371SB has PIIX_UDMA_NONE in the piix_ide_chips[] array,
> so piix_config->flags & PIIX_UDMA is zero, which makes "umul" zero,
> which causes the divide-by-zero on line 334.
> 
>   317	static int piix_set_drive(ide_drive_t *drive, unsigned char speed)
>   318	{
>   319		ide_drive_t *peer = HWIF(drive)->drives + (~drive->dn & 1);
>   320		struct ata_timing t, p;
>   321		int err, T, UT, umul;
>   322	
>   323		if (speed != XFER_PIO_SLOW && speed != drive->current_speed)
>   324			if ((err = ide_config_drive_speed(drive, speed)))
>   325				return err;
>   326	
>   327		umul =  min((speed > XFER_UDMA_4) ? 4 : ((speed > XFER_UDMA_2) ? 2 : 1),
>   328			piix_config->flags & PIIX_UDMA);
>   329	
>   330		if (piix_config->flags & PIIX_VICTORY)
>   331			umul = 2;
>   332	
>   333		T = 1000000000 / piix_clock;
>   334		UT = T / umul;

I think that it should be just sufficient to add the
following test just in front of the offending calculartion.

if (umul == 0)
   ++umul;

Vojtech is this right?


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

* 2.5.7 pre-UDMA PIIX bug
@ 2002-03-29 12:39 Mikael Pettersson
  2002-03-29 14:00 ` Martin Dalecki
  2002-03-29 15:08 ` Vojtech Pavlik
  0 siblings, 2 replies; 7+ messages in thread
From: Mikael Pettersson @ 2002-03-29 12:39 UTC (permalink / raw)
  To: vojtech; +Cc: linux-kernel

Vojtech's version of drivers/ide/piix.c which went into 2.5.7
oopses with a divide-by-zero exception when initialising older
pre-UDMA chips, like in the following 430HX chipset:

00:00.0 Host bridge: Intel Corporation 430HX - 82439HX TXC [Triton II] (rev 03)
00:07.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II] (rev 01)
00:07.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
(PCI IDs 8086:1250, 8086:7000, and 8086:7010, respectively)

The error occurs in piix.c:piix_set_drive() line 334, shown below.
The 82371SB has PIIX_UDMA_NONE in the piix_ide_chips[] array,
so piix_config->flags & PIIX_UDMA is zero, which makes "umul" zero,
which causes the divide-by-zero on line 334.

  317	static int piix_set_drive(ide_drive_t *drive, unsigned char speed)
  318	{
  319		ide_drive_t *peer = HWIF(drive)->drives + (~drive->dn & 1);
  320		struct ata_timing t, p;
  321		int err, T, UT, umul;
  322	
  323		if (speed != XFER_PIO_SLOW && speed != drive->current_speed)
  324			if ((err = ide_config_drive_speed(drive, speed)))
  325				return err;
  326	
  327		umul =  min((speed > XFER_UDMA_4) ? 4 : ((speed > XFER_UDMA_2) ? 2 : 1),
  328			piix_config->flags & PIIX_UDMA);
  329	
  330		if (piix_config->flags & PIIX_VICTORY)
  331			umul = 2;
  332	
  333		T = 1000000000 / piix_clock;
  334		UT = T / umul;

/Mikael

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

end of thread, other threads:[~2002-03-30 14:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-03-29 18:20 2.5.7 pre-UDMA PIIX bug Mikael Pettersson
  -- strict thread matches above, loose matches on Subject: below --
2002-03-29 12:39 Mikael Pettersson
2002-03-29 14:00 ` Martin Dalecki
2002-03-29 15:09   ` Vojtech Pavlik
2002-03-29 15:37   ` Bill Davidsen
2002-03-30 14:26     ` Martin Dalecki
2002-03-29 15:08 ` Vojtech Pavlik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).