Peculiar out-of-sync boot log lines

Peculiar out-of-sync boot log lines

[Nick Warne]

Hi all,

2.6.23.9

I have noticed after applying Bart's patch to word93 blacklist my new
DVD drive:

http://lkml.org/lkml/2007/10/23/475

I see now in logs (look at the hdd line:

[dmesg]
hdc: 39876480 sectors (20416 MB) w/2048KiB Cache, CHS=39560/16/63,
UDMA(66)
hdc: cache flushes not supported
 hdc: hdc1
hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache<7>hdd:
skipping word 93 validity check
, UDMA(66)
Uniform CD-ROM driver Revision: 3.20


<7> ??  And the ", UDMA(66)" gets new lined, so in syslog it appears all
by itself:


[/var/log/syslog]
Nov 29 19:22:00 linuxamd kernel: hda: Maxtor 6Y080L0, ATA DISK drive
Nov 29 19:22:00 linuxamd kernel: hdb: Maxtor 51536H2, ATA DISK drive
Nov 29 19:22:00 linuxamd kernel: ide0 at 0x1f0-0x1f7,0x3f6 on irq 14
Nov 29 19:22:00 linuxamd kernel: hdc: Maxtor 52049H3, ATA DISK drive
Nov 29 19:22:00 linuxamd kernel: hdd: TSSTcorp CDDVDW SH-S202J, ATAPI
CD/DVD-ROM drive
Nov 29 19:22:00 linuxamd kernel: ide1 at 0x170-0x177,0x376 on irq 15
Nov 29 19:22:00 linuxamd kernel: , UDMA(66)



I have tried to trace this, but cannot see anywhere printk
does this.

Nick
-- 
Free Software Foundation Associate Member 5508

Re: Peculiar out-of-sync boot log lines

[Jon Masters]

On Thu, 2007-11-29 at 19:37 +0000, Nick Warne wrote:
> Hi all,
> 
> 2.6.23.9
> 
> I have noticed after applying Bart's patch to word93 blacklist my new
> DVD drive:
> 
> http://lkml.org/lkml/2007/10/23/475
> 
> I see now in logs (look at the hdd line:
> 
> [dmesg]
> hdc: 39876480 sectors (20416 MB) w/2048KiB Cache, CHS=39560/16/63,
> UDMA(66)
> hdc: cache flushes not supported
>  hdc: hdc1
> hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache<7>hdd:
> skipping word 93 validity check
> , UDMA(66)
> Uniform CD-ROM driver Revision: 3.20

Only very early in boot are you guaranteed for things to execute
sequentially, and for logs to look nice and pretty.

Jon.

Re: Peculiar out-of-sync boot log lines

[Nick Warne]

Hi Jon,

On Thu, 29 Nov 2007 14:51:11 -0500
Jon Masters <jonathan@jonmasters.org> wrote:

> 
> On Thu, 2007-11-29 at 19:37 +0000, Nick Warne wrote:
> > Hi all,
> > 
> > 2.6.23.9
> > 
> > I have noticed after applying Bart's patch to word93 blacklist my
> > new DVD drive:
> > 
> > http://lkml.org/lkml/2007/10/23/475
> > 
> > I see now in logs (look at the hdd line:
> > 
> > [dmesg]
> > hdc: 39876480 sectors (20416 MB) w/2048KiB Cache, CHS=39560/16/63,
> > UDMA(66)
> > hdc: cache flushes not supported
> >  hdc: hdc1
> > hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache<7>hdd:
> > skipping word 93 validity check
> > , UDMA(66)
> > Uniform CD-ROM driver Revision: 3.20
> 
> Only very early in boot are you guaranteed for things to execute
> sequentially, and for logs to look nice and pretty.
> 

Yes, but where does the <7> come from?

Nick

-- 
Free Software Foundation Associate Member 5508

Re: Peculiar out-of-sync boot log lines

[Mark Lord]

Nick Warne wrote:
> Hi all,
> 
> 2.6.23.9
> 
> I have noticed after applying Bart's patch to word93 blacklist my new
> DVD drive:
> 
> http://lkml.org/lkml/2007/10/23/475
> 
> I see now in logs (look at the hdd line:
> 
> [dmesg]
> hdc: 39876480 sectors (20416 MB) w/2048KiB Cache, CHS=39560/16/63,
> UDMA(66)
> hdc: cache flushes not supported
>  hdc: hdc1
> hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache<7>hdd:
> skipping word 93 validity check
> , UDMA(66)
> Uniform CD-ROM driver Revision: 3.20
> 
> 
> <7> ??  And the ", UDMA(66)" gets new lined, so in syslog it appears all
> by itself:
...

That's a minor bug with the patch.

The code does this:

ide_dma_verbose::printk( ... "2048kB Cache");
eighty_ninty_three::printk(KERN_DEBUG "%s: skipping word 93 validity check\n");
ide_dma_verbose::printk(", UDMA(66)"

Something in there needs to insert a '\n' before the "skipping word" message.
Since it doesn't do that right now, the KERN_DEBUG string appears as "<7>"

Re: Peculiar out-of-sync boot log lines

[Joe Perches]

On Thu, 2007-11-29 at 20:03 +0000, Nick Warne wrote:
> Yes, but where does the <7> come from?

printk interleaving of functions in ide-cd and ide-iops.

drivers/ide/ide-cd.c ide_cdrom_probe_capabilities could
use something like the string_buf implementations talked
about awhile ago.

Re: Peculiar out-of-sync boot log lines

[Bartlomiej Zolnierkiewicz]

Hi,

On Thursday 29 November 2007, Mark Lord wrote:
> Nick Warne wrote:
> > Hi all,
> > 
> > 2.6.23.9
> > 
> > I have noticed after applying Bart's patch to word93 blacklist my new
> > DVD drive:
> > 
> > http://lkml.org/lkml/2007/10/23/475
> > 
> > I see now in logs (look at the hdd line:
> > 
> > [dmesg]
> > hdc: 39876480 sectors (20416 MB) w/2048KiB Cache, CHS=39560/16/63,
> > UDMA(66)
> > hdc: cache flushes not supported
> >  hdc: hdc1
> > hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache<7>hdd:
> > skipping word 93 validity check
> > , UDMA(66)
> > Uniform CD-ROM driver Revision: 3.20
> > 
> > 
> > <7> ??  And the ", UDMA(66)" gets new lined, so in syslog it appears all
> > by itself:
> ...
> 
> That's a minor bug with the patch.
> 
> The code does this:
> 
> ide_dma_verbose::printk( ... "2048kB Cache");
> eighty_ninty_three::printk(KERN_DEBUG "%s: skipping word 93 validity check\n");
> ide_dma_verbose::printk(", UDMA(66)"

Thanks for reporting/debugging it guys!

> Something in there needs to insert a '\n' before the "skipping word" message.
> Since it doesn't do that right now, the KERN_DEBUG string appears as "<7>"

This seems like a good occasion to fix ide_dma_verbose() for good so... :)

[ patch is against current Linus tree so might not apply to 2.6.23.9 ]

[PATCH] ide: DMA reporting and validity checking fixes

* ide_xfer_verbose() fixups:
  - beautify returned mode names
  - fix PIO5 reporting
  - make it return 'const char *'

* Change printk() level from KERN_DEBUG to KERN_INFO in ide_find_dma_mode().

* Add ide_id_dma_bug() helper based on ide_dma_verbose() to check for invalid
  DMA info in identify block.

* Use ide_id_dma_bug() in ide_tune_dma() and ide_driveid_update().

  As a result DMA won't be tuned or will be disabled after tuning if device
  reports inconsistent info about enabled DMA mode (ide_dma_verbose() does the
  same checks while the IDE device is probed by ide-{cd,disk} device driver).

* Since (id->capability & 1) && id->tDMA is a valid configuration handle
  it correctly in ide_id_dma_bug().

* Remove no longer needed ide_dma_verbose().

This patch should fix the following problem with out-of-sync IDE messages
reported by Nick Warned:

       hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache<7>hdd:
       skipping word 93 validity check
        , UDMA(66)

and later debugged by Mark Lord to be caused by:

        ide_dma_verbose()
                printk( ... "2048kB Cache");
        eighty_ninty_three()
                printk(KERN_DEBUG "%s: skipping word 93 validity check\n");
        ide_dma_verbose()
                printk(", UDMA(66)"

Please note that as a result ide-{cd,disk} device drivers won't report the
DMA speed used but this is intended since now DMA mode being used is always
reported by IDE core code.

Cc: Nick Warne <nick@ukfsn.org>
Cc: Mark Lord <lkml@rtr.ca>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-cd.c   |    7 ------
 drivers/ide/ide-disk.c |    5 ----
 drivers/ide/ide-dma.c  |   57 ++++++++++++-------------------------------------
 drivers/ide/ide-iops.c |    3 ++
 drivers/ide/ide-lib.c  |   55 ++++++++++++++++++++++++-----------------------
 include/linux/ide.h    |    6 ++---
 6 files changed, 51 insertions(+), 82 deletions(-)

Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -3049,12 +3049,7 @@ int ide_cdrom_probe_capabilities (ide_dr
         else 	
         	printk(" drive");
 
-	printk(", %dkB Cache", be16_to_cpu(cap.buffer_size));
-
-	if (drive->using_dma)
-		ide_dma_verbose(drive);
-
-	printk("\n");
+	printk(", %dkB Cache\n", be16_to_cpu(cap.buffer_size));
 
 	return nslots;
 }
Index: b/drivers/ide/ide-disk.c
===================================================================
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -961,11 +961,8 @@ static void idedisk_setup (ide_drive_t *
 	if (id->buf_size)
 		printk (" w/%dKiB Cache", id->buf_size/2);
 
-	printk(", CHS=%d/%d/%d", 
+	printk(", CHS=%d/%d/%d\n",
 	       drive->bios_cyl, drive->bios_head, drive->bios_sect);
-	if (drive->using_dma)
-		ide_dma_verbose(drive);
-	printk("\n");
 
 	/* write cache enabled? */
 	if ((id->csfo & 1) || (id->cfs_enable_1 & (1 << 5)))
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -753,7 +753,7 @@ u8 ide_find_dma_mode(ide_drive_t *drive,
 			mode = XFER_MW_DMA_1;
 	}
 
-	printk(KERN_DEBUG "%s: %s mode selected\n", drive->name,
+	printk(KERN_INFO "%s: %s mode selected\n", drive->name,
 			  mode ? ide_xfer_verbose(mode) : "no DMA");
 
 	return min(mode, req_mode);
@@ -772,6 +772,9 @@ static int ide_tune_dma(ide_drive_t *dri
 	if (__ide_dma_bad_drive(drive))
 		return 0;
 
+	if (ide_id_dma_bug(drive))
+		return 0;
+
 	if (drive->hwif->host_flags & IDE_HFLAG_TRUST_BIOS_FOR_DMA)
 		return config_drive_for_dma(drive);
 
@@ -806,58 +809,26 @@ static int ide_dma_check(ide_drive_t *dr
 	return vdma ? 0 : -1;
 }
 
-void ide_dma_verbose(ide_drive_t *drive)
+int ide_id_dma_bug(ide_drive_t *drive)
 {
-	struct hd_driveid *id	= drive->id;
-	ide_hwif_t *hwif	= HWIF(drive);
+	struct hd_driveid *id = drive->id;
 
 	if (id->field_valid & 4) {
 		if ((id->dma_ultra >> 8) && (id->dma_mword >> 8))
-			goto bug_dma_off;
-		if (id->dma_ultra & ((id->dma_ultra >> 8) & hwif->ultra_mask)) {
-			if (((id->dma_ultra >> 11) & 0x1F) &&
-			    eighty_ninty_three(drive)) {
-				if ((id->dma_ultra >> 15) & 1) {
-					printk(", UDMA(mode 7)");
-				} else if ((id->dma_ultra >> 14) & 1) {
-					printk(", UDMA(133)");
-				} else if ((id->dma_ultra >> 13) & 1) {
-					printk(", UDMA(100)");
-				} else if ((id->dma_ultra >> 12) & 1) {
-					printk(", UDMA(66)");
-				} else if ((id->dma_ultra >> 11) & 1) {
-					printk(", UDMA(44)");
-				} else
-					goto mode_two;
-			} else {
-		mode_two:
-				if ((id->dma_ultra >> 10) & 1) {
-					printk(", UDMA(33)");
-				} else if ((id->dma_ultra >> 9) & 1) {
-					printk(", UDMA(25)");
-				} else if ((id->dma_ultra >> 8) & 1) {
-					printk(", UDMA(16)");
-				}
-			}
-		} else {
-			printk(", (U)DMA");	/* Can be BIOS-enabled! */
-		}
+			goto err_out;
 	} else if (id->field_valid & 2) {
 		if ((id->dma_mword >> 8) && (id->dma_1word >> 8))
-			goto bug_dma_off;
-		printk(", DMA");
+			goto err_out;
 	} else if (id->field_valid & 1) {
-		goto bug_dma_off;
+		if (id->tDMA == 0)
+			goto err_out;
 	}
-	return;
-bug_dma_off:
-	printk(", BUG DMA OFF");
-	hwif->dma_off_quietly(drive);
-	return;
+	return 0;
+err_out:
+	printk(KERN_ERR "%s: bad DMA info in identify block\n", drive->name);
+	return 1;
 }
 
-EXPORT_SYMBOL(ide_dma_verbose);
-
 int ide_set_dma(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -748,6 +748,9 @@ int ide_driveid_update(ide_drive_t *driv
 		drive->id->dma_1word = id->dma_1word;
 		/* anything more ? */
 		kfree(id);
+
+		if (drive->using_dma && ide_id_dma_bug(drive))
+			ide_dma_off(drive);
 	}
 
 	return 1;
Index: b/drivers/ide/ide-lib.c
===================================================================
--- a/drivers/ide/ide-lib.c
+++ b/drivers/ide/ide-lib.c
@@ -29,41 +29,44 @@
  *	Add common non I/O op stuff here. Make sure it has proper
  *	kernel-doc function headers or your patch will be rejected
  */
- 
+
+static const char *udma_str[] =
+	 { "UDMA/16", "UDMA/25",  "UDMA/33",  "UDMA/44",
+	   "UDMA/66", "UDMA/100", "UDMA/133", "UDMA7" };
+static const char *mwdma_str[] =
+	{ "MWDMA0", "MWDMA1", "MWDMA2" };
+static const char *swdma_str[] =
+	{ "SWDMA0", "SWDMA1", "SWDMA2" };
+static const char *pio_str[] =
+	{ "PIO0", "PIO1", "PIO2", "PIO3", "PIO4", "PIO5" };
 
 /**
  *	ide_xfer_verbose	-	return IDE mode names
- *	@xfer_rate: rate to name
+ *	@mode: transfer mode
  *
  *	Returns a constant string giving the name of the mode
  *	requested.
  */
 
-char *ide_xfer_verbose (u8 xfer_rate)
+const char *ide_xfer_verbose(u8 mode)
 {
-        switch(xfer_rate) {
-                case XFER_UDMA_7:	return("UDMA 7");
-                case XFER_UDMA_6:	return("UDMA 6");
-                case XFER_UDMA_5:	return("UDMA 5");
-                case XFER_UDMA_4:	return("UDMA 4");
-                case XFER_UDMA_3:	return("UDMA 3");
-                case XFER_UDMA_2:	return("UDMA 2");
-                case XFER_UDMA_1:	return("UDMA 1");
-                case XFER_UDMA_0:	return("UDMA 0");
-                case XFER_MW_DMA_2:	return("MW DMA 2");
-                case XFER_MW_DMA_1:	return("MW DMA 1");
-                case XFER_MW_DMA_0:	return("MW DMA 0");
-                case XFER_SW_DMA_2:	return("SW DMA 2");
-                case XFER_SW_DMA_1:	return("SW DMA 1");
-                case XFER_SW_DMA_0:	return("SW DMA 0");
-                case XFER_PIO_4:	return("PIO 4");
-                case XFER_PIO_3:	return("PIO 3");
-                case XFER_PIO_2:	return("PIO 2");
-                case XFER_PIO_1:	return("PIO 1");
-                case XFER_PIO_0:	return("PIO 0");
-                case XFER_PIO_SLOW:	return("PIO SLOW");
-                default:		return("XFER ERROR");
-        }
+	const char *s;
+	u8 i = mode & 0xf;
+
+	if (mode >= XFER_UDMA_0 && mode <= XFER_UDMA_7)
+		s = udma_str[i];
+	else if (mode >= XFER_MW_DMA_0 && mode <= XFER_MW_DMA_2)
+		s = mwdma_str[i];
+	else if (mode >= XFER_SW_DMA_0 && mode <= XFER_SW_DMA_2)
+		s = swdma_str[i];
+	else if (mode >= XFER_PIO_0 && mode <= XFER_PIO_5)
+		s = pio_str[i & 0x7];
+	else if (mode == XFER_PIO_SLOW)
+		s = "XFER SLOW";
+	else
+		s = "XFER ERROR";
+
+	return s;
 }
 
 EXPORT_SYMBOL(ide_xfer_verbose);
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1255,6 +1255,7 @@ int ide_in_drive_list(struct hd_driveid 
 
 #ifdef CONFIG_BLK_DEV_IDEDMA
 int __ide_dma_bad_drive(ide_drive_t *);
+int ide_id_dma_bug(ide_drive_t *);
 
 u8 ide_find_dma_mode(ide_drive_t *, u8);
 
@@ -1264,7 +1265,6 @@ static inline u8 ide_max_dma_mode(ide_dr
 }
 
 void ide_dma_off(ide_drive_t *);
-void ide_dma_verbose(ide_drive_t *);
 int ide_set_dma(ide_drive_t *);
 ide_startstop_t ide_dma_intr(ide_drive_t *);
 
@@ -1287,6 +1287,7 @@ extern void ide_dma_timeout(ide_drive_t 
 #endif /* CONFIG_BLK_DEV_IDEDMA_PCI */
 
 #else
+static inline int ide_id_dma_bug(ide_drive_t *drive) { return 0; }
 static inline u8 ide_find_dma_mode(ide_drive_t *drive, u8 speed) { return 0; }
 static inline u8 ide_max_dma_mode(ide_drive_t *drive) { return 0; }
 static inline void ide_dma_off(ide_drive_t *drive) { ; }
@@ -1333,8 +1334,7 @@ static inline void ide_set_hwifdata (ide
 	hwif->hwif_data = data;
 }
 
-/* ide-lib.c */
-extern char *ide_xfer_verbose(u8 xfer_rate);
+const char *ide_xfer_verbose(u8);
 extern void ide_toggle_bounce(ide_drive_t *drive, int on);
 extern int ide_set_xfer_rate(ide_drive_t *drive, u8 rate);
 

Re: Peculiar out-of-sync boot log lines

[Randy Dunlap]

On Sat, 1 Dec 2007 22:59:35 +0100 Bartlomiej Zolnierkiewicz wrote:

> Thanks for reporting/debugging it guys!
> 
> > Something in there needs to insert a '\n' before the "skipping word" message.
> > Since it doesn't do that right now, the KERN_DEBUG string appears as "<7>"
> 
> This seems like a good occasion to fix ide_dma_verbose() for good so... :)
> 
> [ patch is against current Linus tree so might not apply to 2.6.23.9 ]
> 
> [PATCH] ide: DMA reporting and validity checking fixes
> 
> * ide_xfer_verbose() fixups:
>   - beautify returned mode names
>   - fix PIO5 reporting
>   - make it return 'const char *'
> 
> * Change printk() level from KERN_DEBUG to KERN_INFO in ide_find_dma_mode().
> 
> * Add ide_id_dma_bug() helper based on ide_dma_verbose() to check for invalid
>   DMA info in identify block.
> 
> * Use ide_id_dma_bug() in ide_tune_dma() and ide_driveid_update().
> 
>   As a result DMA won't be tuned or will be disabled after tuning if device
>   reports inconsistent info about enabled DMA mode (ide_dma_verbose() does the
>   same checks while the IDE device is probed by ide-{cd,disk} device driver).
> 
> * Since (id->capability & 1) && id->tDMA is a valid configuration handle
>   it correctly in ide_id_dma_bug().
> 
> * Remove no longer needed ide_dma_verbose().
> 
> This patch should fix the following problem with out-of-sync IDE messages
> reported by Nick Warned:
> 
>        hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache<7>hdd:
>        skipping word 93 validity check
>         , UDMA(66)
> 
> and later debugged by Mark Lord to be caused by:
> 
>         ide_dma_verbose()
>                 printk( ... "2048kB Cache");
>         eighty_ninty_three()
>                 printk(KERN_DEBUG "%s: skipping word 93 validity check\n");
>         ide_dma_verbose()
>                 printk(", UDMA(66)"
> 
> Please note that as a result ide-{cd,disk} device drivers won't report the
> DMA speed used but this is intended since now DMA mode being used is always
> reported by IDE core code.
> 
> Cc: Nick Warne <nick@ukfsn.org>
> Cc: Mark Lord <lkml@rtr.ca>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> ---
>  drivers/ide/ide-cd.c   |    7 ------
>  drivers/ide/ide-disk.c |    5 ----
>  drivers/ide/ide-dma.c  |   57 ++++++++++++-------------------------------------
>  drivers/ide/ide-iops.c |    3 ++
>  drivers/ide/ide-lib.c  |   55 ++++++++++++++++++++++++-----------------------
>  include/linux/ide.h    |    6 ++---
>  6 files changed, 51 insertions(+), 82 deletions(-)
> 
> Index: b/drivers/ide/ide-cd.c
> ===================================================================
> --- a/drivers/ide/ide-cd.c
> +++ b/drivers/ide/ide-cd.c
> @@ -3049,12 +3049,7 @@ int ide_cdrom_probe_capabilities (ide_dr
>          else 	
>          	printk(" drive");
>  
> -	printk(", %dkB Cache", be16_to_cpu(cap.buffer_size));
> -
> -	if (drive->using_dma)
> -		ide_dma_verbose(drive);
> -
> -	printk("\n");
> +	printk(", %dkB Cache\n", be16_to_cpu(cap.buffer_size));

Use
	printk(KERN_CONT ...
so that someone won't try to add another KERN_* facility level to it.
(same for other continued printk calls in this patch)

>  
>  	return nslots;
>  }
> Index: b/drivers/ide/ide-disk.c
> ===================================================================
> --- a/drivers/ide/ide-disk.c
> +++ b/drivers/ide/ide-disk.c
> @@ -961,11 +961,8 @@ static void idedisk_setup (ide_drive_t *
>  	if (id->buf_size)
>  		printk (" w/%dKiB Cache", id->buf_size/2);
>  
> -	printk(", CHS=%d/%d/%d", 
> +	printk(", CHS=%d/%d/%d\n",

Ditto.

>  	       drive->bios_cyl, drive->bios_head, drive->bios_sect);
> -	if (drive->using_dma)
> -		ide_dma_verbose(drive);
> -	printk("\n");
>  
>  	/* write cache enabled? */
>  	if ((id->csfo & 1) || (id->cfs_enable_1 & (1 << 5)))

> Index: b/include/linux/ide.h
> ===================================================================
> --- a/include/linux/ide.h
> +++ b/include/linux/ide.h

> @@ -1333,8 +1334,7 @@ static inline void ide_set_hwifdata (ide
>  	hwif->hwif_data = data;
>  }
>  
> -/* ide-lib.c */
> -extern char *ide_xfer_verbose(u8 xfer_rate);
> +const char *ide_xfer_verbose(u8);
>  extern void ide_toggle_bounce(ide_drive_t *drive, int on);
>  extern int ide_set_xfer_rate(ide_drive_t *drive, u8 rate);

Ideally function prototypes would include variable names, not just
types, as a helpful hint to readers of them.

---
~Randy
The Linux kernel requires that any needed documentation accompany all
changes requiring said documentation -- part of the source-code patch
must apply to the Documentation/ directory. [...]
To sum up: No undocumented changes.
-- Donnie Berkholz engages in some wishful thinking
<http://lwn.net/Articles/259470/> Quote of the Week

Re: Peculiar out-of-sync boot log lines

[Nick Warne]

On Sat, 1 Dec 2007 22:59:35 +0100
Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:

 
> Thanks for reporting/debugging it guys!
> 
> > Something in there needs to insert a '\n' before the "skipping
> > word" message. Since it doesn't do that right now, the KERN_DEBUG
> > string appears as "<7>"
> 
> This seems like a good occasion to fix ide_dma_verbose() for good
> so... :)
 
> This patch should fix the following problem with out-of-sync IDE
> messages reported by Nick Warned:
...

I was only reporting it... not warning anybody ;-)

Nick
-- 
Free Software Foundation Associate Member 5508

Re: Peculiar out-of-sync boot log lines

[Bartlomiej Zolnierkiewicz]

On Sunday 02 December 2007, Randy Dunlap wrote:
> On Sat, 1 Dec 2007 22:59:35 +0100 Bartlomiej Zolnierkiewicz wrote:
> 
> > Thanks for reporting/debugging it guys!
> > 
> > > Something in there needs to insert a '\n' before the "skipping word" message.
> > > Since it doesn't do that right now, the KERN_DEBUG string appears as "<7>"
> > 
> > This seems like a good occasion to fix ide_dma_verbose() for good so... :)
> > 
> > [ patch is against current Linus tree so might not apply to 2.6.23.9 ]
> > 
> > [PATCH] ide: DMA reporting and validity checking fixes
> > 
> > * ide_xfer_verbose() fixups:
> >   - beautify returned mode names
> >   - fix PIO5 reporting
> >   - make it return 'const char *'
> > 
> > * Change printk() level from KERN_DEBUG to KERN_INFO in ide_find_dma_mode().
> > 
> > * Add ide_id_dma_bug() helper based on ide_dma_verbose() to check for invalid
> >   DMA info in identify block.
> > 
> > * Use ide_id_dma_bug() in ide_tune_dma() and ide_driveid_update().
> > 
> >   As a result DMA won't be tuned or will be disabled after tuning if device
> >   reports inconsistent info about enabled DMA mode (ide_dma_verbose() does the
> >   same checks while the IDE device is probed by ide-{cd,disk} device driver).
> > 
> > * Since (id->capability & 1) && id->tDMA is a valid configuration handle
> >   it correctly in ide_id_dma_bug().
> > 
> > * Remove no longer needed ide_dma_verbose().
> > 
> > This patch should fix the following problem with out-of-sync IDE messages
> > reported by Nick Warned:
> > 
> >        hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache<7>hdd:
> >        skipping word 93 validity check
> >         , UDMA(66)
> > 
> > and later debugged by Mark Lord to be caused by:
> > 
> >         ide_dma_verbose()
> >                 printk( ... "2048kB Cache");
> >         eighty_ninty_three()
> >                 printk(KERN_DEBUG "%s: skipping word 93 validity check\n");
> >         ide_dma_verbose()
> >                 printk(", UDMA(66)"
> > 
> > Please note that as a result ide-{cd,disk} device drivers won't report the
> > DMA speed used but this is intended since now DMA mode being used is always
> > reported by IDE core code.
> > 
> > Cc: Nick Warne <nick@ukfsn.org>
> > Cc: Mark Lord <lkml@rtr.ca>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > ---
> >  drivers/ide/ide-cd.c   |    7 ------
> >  drivers/ide/ide-disk.c |    5 ----
> >  drivers/ide/ide-dma.c  |   57 ++++++++++++-------------------------------------
> >  drivers/ide/ide-iops.c |    3 ++
> >  drivers/ide/ide-lib.c  |   55 ++++++++++++++++++++++++-----------------------
> >  include/linux/ide.h    |    6 ++---
> >  6 files changed, 51 insertions(+), 82 deletions(-)
> > 
> > Index: b/drivers/ide/ide-cd.c
> > ===================================================================
> > --- a/drivers/ide/ide-cd.c
> > +++ b/drivers/ide/ide-cd.c
> > @@ -3049,12 +3049,7 @@ int ide_cdrom_probe_capabilities (ide_dr
> >          else 	
> >          	printk(" drive");
> >  
> > -	printk(", %dkB Cache", be16_to_cpu(cap.buffer_size));
> > -
> > -	if (drive->using_dma)
> > -		ide_dma_verbose(drive);
> > -
> > -	printk("\n");
> > +	printk(", %dkB Cache\n", be16_to_cpu(cap.buffer_size));
> 
> Use
> 	printk(KERN_CONT ...
> so that someone won't try to add another KERN_* facility level to it.
> (same for other continued printk calls in this patch)

good idea, fixed

[ I keep forgetting that there is KERN_CONT... ]

> >  
> >  	return nslots;
> >  }
> > Index: b/drivers/ide/ide-disk.c
> > ===================================================================
> > --- a/drivers/ide/ide-disk.c
> > +++ b/drivers/ide/ide-disk.c
> > @@ -961,11 +961,8 @@ static void idedisk_setup (ide_drive_t *
> >  	if (id->buf_size)
> >  		printk (" w/%dKiB Cache", id->buf_size/2);
> >  
> > -	printk(", CHS=%d/%d/%d", 
> > +	printk(", CHS=%d/%d/%d\n",
> 
> Ditto.

fixed

> >  	       drive->bios_cyl, drive->bios_head, drive->bios_sect);
> > -	if (drive->using_dma)
> > -		ide_dma_verbose(drive);
> > -	printk("\n");
> >  
> >  	/* write cache enabled? */
> >  	if ((id->csfo & 1) || (id->cfs_enable_1 & (1 << 5)))
> 
> > Index: b/include/linux/ide.h
> > ===================================================================
> > --- a/include/linux/ide.h
> > +++ b/include/linux/ide.h
> 
> > @@ -1333,8 +1334,7 @@ static inline void ide_set_hwifdata (ide
> >  	hwif->hwif_data = data;
> >  }
> >  
> > -/* ide-lib.c */
> > -extern char *ide_xfer_verbose(u8 xfer_rate);
> > +const char *ide_xfer_verbose(u8);
> >  extern void ide_toggle_bounce(ide_drive_t *drive, int on);
> >  extern int ide_set_xfer_rate(ide_drive_t *drive, u8 rate);
> 
> Ideally function prototypes would include variable names, not just
> types, as a helpful hint to readers of them.

fixed

Thanks Randy!

[PATCH] ide: DMA reporting and validity checking fixes (take 2)

* ide_xfer_verbose() fixups:
  - beautify returned mode names
  - fix PIO5 reporting
  - make it return 'const char *'

* Change printk() level from KERN_DEBUG to KERN_INFO in ide_find_dma_mode().

* Add ide_id_dma_bug() helper based on ide_dma_verbose() to check for invalid
  DMA info in identify block.

* Use ide_id_dma_bug() in ide_tune_dma() and ide_driveid_update().

  As a result DMA won't be tuned or will be disabled after tuning if device
  reports inconsistent info about enabled DMA mode (ide_dma_verbose() does the
  same checks while the IDE device is probed by ide-{cd,disk} device driver).

* Since (id->capability & 1) && id->tDMA is a valid configuration handle
  it correctly in ide_id_dma_bug().

* Remove no longer needed ide_dma_verbose().

This patch should fix the following problem with out-of-sync IDE messages
reported by Nick Warne:

       hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache<7>hdd:
       skipping word 93 validity check
        , UDMA(66)

and later debugged by Mark Lord to be caused by:

        ide_dma_verbose()
                printk( ... "2048kB Cache");
        eighty_ninty_three()
                printk(KERN_DEBUG "%s: skipping word 93 validity check\n");
        ide_dma_verbose()
                printk(", UDMA(66)"

Please note that as a result ide-{cd,disk} device drivers won't report the
DMA speed used but this is intended since now DMA mode being used is always
reported by IDE core code.

v2:
* fixes suggested by Randy:
  - use KERN_CONT for printk()-s in ide-{cd,disk}.c
  - don't remove argument name from ide_xfer_verbose() declaration

Cc: Nick Warne <nick@ukfsn.org>
Cc: Mark Lord <lkml@rtr.ca>
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-cd.c   |    7 ------
 drivers/ide/ide-disk.c |    7 +-----
 drivers/ide/ide-dma.c  |   57 ++++++++++++-------------------------------------
 drivers/ide/ide-iops.c |    3 ++
 drivers/ide/ide-lib.c  |   55 ++++++++++++++++++++++++-----------------------
 include/linux/ide.h    |    6 ++---
 6 files changed, 52 insertions(+), 83 deletions(-)

Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -3049,12 +3049,7 @@ int ide_cdrom_probe_capabilities (ide_dr
         else 	
         	printk(" drive");
 
-	printk(", %dkB Cache", be16_to_cpu(cap.buffer_size));
-
-	if (drive->using_dma)
-		ide_dma_verbose(drive);
-
-	printk("\n");
+	printk(KERN_CONT ", %dkB Cache\n", be16_to_cpu(cap.buffer_size));
 
 	return nslots;
 }
Index: b/drivers/ide/ide-disk.c
===================================================================
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -961,11 +961,8 @@ static void idedisk_setup (ide_drive_t *
 	if (id->buf_size)
 		printk (" w/%dKiB Cache", id->buf_size/2);
 
-	printk(", CHS=%d/%d/%d", 
-	       drive->bios_cyl, drive->bios_head, drive->bios_sect);
-	if (drive->using_dma)
-		ide_dma_verbose(drive);
-	printk("\n");
+	printk(KERN_CONT ", CHS=%d/%d/%d\n",
+			 drive->bios_cyl, drive->bios_head, drive->bios_sect);
 
 	/* write cache enabled? */
 	if ((id->csfo & 1) || (id->cfs_enable_1 & (1 << 5)))
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -753,7 +753,7 @@ u8 ide_find_dma_mode(ide_drive_t *drive,
 			mode = XFER_MW_DMA_1;
 	}
 
-	printk(KERN_DEBUG "%s: %s mode selected\n", drive->name,
+	printk(KERN_INFO "%s: %s mode selected\n", drive->name,
 			  mode ? ide_xfer_verbose(mode) : "no DMA");
 
 	return min(mode, req_mode);
@@ -772,6 +772,9 @@ static int ide_tune_dma(ide_drive_t *dri
 	if (__ide_dma_bad_drive(drive))
 		return 0;
 
+	if (ide_id_dma_bug(drive))
+		return 0;
+
 	if (drive->hwif->host_flags & IDE_HFLAG_TRUST_BIOS_FOR_DMA)
 		return config_drive_for_dma(drive);
 
@@ -806,58 +809,26 @@ static int ide_dma_check(ide_drive_t *dr
 	return vdma ? 0 : -1;
 }
 
-void ide_dma_verbose(ide_drive_t *drive)
+int ide_id_dma_bug(ide_drive_t *drive)
 {
-	struct hd_driveid *id	= drive->id;
-	ide_hwif_t *hwif	= HWIF(drive);
+	struct hd_driveid *id = drive->id;
 
 	if (id->field_valid & 4) {
 		if ((id->dma_ultra >> 8) && (id->dma_mword >> 8))
-			goto bug_dma_off;
-		if (id->dma_ultra & ((id->dma_ultra >> 8) & hwif->ultra_mask)) {
-			if (((id->dma_ultra >> 11) & 0x1F) &&
-			    eighty_ninty_three(drive)) {
-				if ((id->dma_ultra >> 15) & 1) {
-					printk(", UDMA(mode 7)");
-				} else if ((id->dma_ultra >> 14) & 1) {
-					printk(", UDMA(133)");
-				} else if ((id->dma_ultra >> 13) & 1) {
-					printk(", UDMA(100)");
-				} else if ((id->dma_ultra >> 12) & 1) {
-					printk(", UDMA(66)");
-				} else if ((id->dma_ultra >> 11) & 1) {
-					printk(", UDMA(44)");
-				} else
-					goto mode_two;
-			} else {
-		mode_two:
-				if ((id->dma_ultra >> 10) & 1) {
-					printk(", UDMA(33)");
-				} else if ((id->dma_ultra >> 9) & 1) {
-					printk(", UDMA(25)");
-				} else if ((id->dma_ultra >> 8) & 1) {
-					printk(", UDMA(16)");
-				}
-			}
-		} else {
-			printk(", (U)DMA");	/* Can be BIOS-enabled! */
-		}
+			goto err_out;
 	} else if (id->field_valid & 2) {
 		if ((id->dma_mword >> 8) && (id->dma_1word >> 8))
-			goto bug_dma_off;
-		printk(", DMA");
+			goto err_out;
 	} else if (id->field_valid & 1) {
-		goto bug_dma_off;
+		if (id->tDMA == 0)
+			goto err_out;
 	}
-	return;
-bug_dma_off:
-	printk(", BUG DMA OFF");
-	hwif->dma_off_quietly(drive);
-	return;
+	return 0;
+err_out:
+	printk(KERN_ERR "%s: bad DMA info in identify block\n", drive->name);
+	return 1;
 }
 
-EXPORT_SYMBOL(ide_dma_verbose);
-
 int ide_set_dma(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -748,6 +748,9 @@ int ide_driveid_update(ide_drive_t *driv
 		drive->id->dma_1word = id->dma_1word;
 		/* anything more ? */
 		kfree(id);
+
+		if (drive->using_dma && ide_id_dma_bug(drive))
+			ide_dma_off(drive);
 	}
 
 	return 1;
Index: b/drivers/ide/ide-lib.c
===================================================================
--- a/drivers/ide/ide-lib.c
+++ b/drivers/ide/ide-lib.c
@@ -29,41 +29,44 @@
  *	Add common non I/O op stuff here. Make sure it has proper
  *	kernel-doc function headers or your patch will be rejected
  */
- 
+
+static const char *udma_str[] =
+	 { "UDMA/16", "UDMA/25",  "UDMA/33",  "UDMA/44",
+	   "UDMA/66", "UDMA/100", "UDMA/133", "UDMA7" };
+static const char *mwdma_str[] =
+	{ "MWDMA0", "MWDMA1", "MWDMA2" };
+static const char *swdma_str[] =
+	{ "SWDMA0", "SWDMA1", "SWDMA2" };
+static const char *pio_str[] =
+	{ "PIO0", "PIO1", "PIO2", "PIO3", "PIO4", "PIO5" };
 
 /**
  *	ide_xfer_verbose	-	return IDE mode names
- *	@xfer_rate: rate to name
+ *	@mode: transfer mode
  *
  *	Returns a constant string giving the name of the mode
  *	requested.
  */
 
-char *ide_xfer_verbose (u8 xfer_rate)
+const char *ide_xfer_verbose(u8 mode)
 {
-        switch(xfer_rate) {
-                case XFER_UDMA_7:	return("UDMA 7");
-                case XFER_UDMA_6:	return("UDMA 6");
-                case XFER_UDMA_5:	return("UDMA 5");
-                case XFER_UDMA_4:	return("UDMA 4");
-                case XFER_UDMA_3:	return("UDMA 3");
-                case XFER_UDMA_2:	return("UDMA 2");
-                case XFER_UDMA_1:	return("UDMA 1");
-                case XFER_UDMA_0:	return("UDMA 0");
-                case XFER_MW_DMA_2:	return("MW DMA 2");
-                case XFER_MW_DMA_1:	return("MW DMA 1");
-                case XFER_MW_DMA_0:	return("MW DMA 0");
-                case XFER_SW_DMA_2:	return("SW DMA 2");
-                case XFER_SW_DMA_1:	return("SW DMA 1");
-                case XFER_SW_DMA_0:	return("SW DMA 0");
-                case XFER_PIO_4:	return("PIO 4");
-                case XFER_PIO_3:	return("PIO 3");
-                case XFER_PIO_2:	return("PIO 2");
-                case XFER_PIO_1:	return("PIO 1");
-                case XFER_PIO_0:	return("PIO 0");
-                case XFER_PIO_SLOW:	return("PIO SLOW");
-                default:		return("XFER ERROR");
-        }
+	const char *s;
+	u8 i = mode & 0xf;
+
+	if (mode >= XFER_UDMA_0 && mode <= XFER_UDMA_7)
+		s = udma_str[i];
+	else if (mode >= XFER_MW_DMA_0 && mode <= XFER_MW_DMA_2)
+		s = mwdma_str[i];
+	else if (mode >= XFER_SW_DMA_0 && mode <= XFER_SW_DMA_2)
+		s = swdma_str[i];
+	else if (mode >= XFER_PIO_0 && mode <= XFER_PIO_5)
+		s = pio_str[i & 0x7];
+	else if (mode == XFER_PIO_SLOW)
+		s = "XFER SLOW";
+	else
+		s = "XFER ERROR";
+
+	return s;
 }
 
 EXPORT_SYMBOL(ide_xfer_verbose);
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1255,6 +1255,7 @@ int ide_in_drive_list(struct hd_driveid 
 
 #ifdef CONFIG_BLK_DEV_IDEDMA
 int __ide_dma_bad_drive(ide_drive_t *);
+int ide_id_dma_bug(ide_drive_t *);
 
 u8 ide_find_dma_mode(ide_drive_t *, u8);
 
@@ -1264,7 +1265,6 @@ static inline u8 ide_max_dma_mode(ide_dr
 }
 
 void ide_dma_off(ide_drive_t *);
-void ide_dma_verbose(ide_drive_t *);
 int ide_set_dma(ide_drive_t *);
 ide_startstop_t ide_dma_intr(ide_drive_t *);
 
@@ -1287,6 +1287,7 @@ extern void ide_dma_timeout(ide_drive_t 
 #endif /* CONFIG_BLK_DEV_IDEDMA_PCI */
 
 #else
+static inline int ide_id_dma_bug(ide_drive_t *drive) { return 0; }
 static inline u8 ide_find_dma_mode(ide_drive_t *drive, u8 speed) { return 0; }
 static inline u8 ide_max_dma_mode(ide_drive_t *drive) { return 0; }
 static inline void ide_dma_off(ide_drive_t *drive) { ; }
@@ -1333,8 +1334,7 @@ static inline void ide_set_hwifdata (ide
 	hwif->hwif_data = data;
 }
 
-/* ide-lib.c */
-extern char *ide_xfer_verbose(u8 xfer_rate);
+const char *ide_xfer_verbose(u8 mode);
 extern void ide_toggle_bounce(ide_drive_t *drive, int on);
 extern int ide_set_xfer_rate(ide_drive_t *drive, u8 rate);
 

Re: Peculiar out-of-sync boot log lines

[Bartlomiej Zolnierkiewicz]

On Sunday 02 December 2007, Nick Warne wrote:
> On Sat, 1 Dec 2007 22:59:35 +0100
> Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> 
>  
> > Thanks for reporting/debugging it guys!
> > 
> > > Something in there needs to insert a '\n' before the "skipping
> > > word" message. Since it doesn't do that right now, the KERN_DEBUG
> > > string appears as "<7>"
> > 
> > This seems like a good occasion to fix ide_dma_verbose() for good
> > so... :)
>  
> > This patch should fix the following problem with out-of-sync IDE
> > messages reported by Nick Warned:
> ...

Sorry for that, I fixed it in "take 2" of the patch.

> I was only reporting it... not warning anybody ;-)

Hehe... :-)

Bart

Re: Peculiar out-of-sync boot log lines

[Sergei Shtylyov]

Hello.

Bartlomiej Zolnierkiewicz wrote:

> [PATCH] ide: DMA reporting and validity checking fixes (take 2)

> * ide_xfer_verbose() fixups:
>   - beautify returned mode names
>   - fix PIO5 reporting
>   - make it return 'const char *'

> * Change printk() level from KERN_DEBUG to KERN_INFO in ide_find_dma_mode().

> * Add ide_id_dma_bug() helper based on ide_dma_verbose() to check for invalid
>   DMA info in identify block.

> * Use ide_id_dma_bug() in ide_tune_dma() and ide_driveid_update().

>   As a result DMA won't be tuned or will be disabled after tuning if device
>   reports inconsistent info about enabled DMA mode (ide_dma_verbose() does the
>   same checks while the IDE device is probed by ide-{cd,disk} device driver).

> * Since (id->capability & 1) && id->tDMA is a valid configuration handle
>   it correctly in ide_id_dma_bug().

    Huh? You don't check (id->capability & 1) there...

> * Remove no longer needed ide_dma_verbose().

> This patch should fix the following problem with out-of-sync IDE messages
> reported by Nick Warne:

>        hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache<7>hdd:
>        skipping word 93 validity check
>         , UDMA(66)

> and later debugged by Mark Lord to be caused by:

>         ide_dma_verbose()
>                 printk( ... "2048kB Cache");
>         eighty_ninty_three()
>                 printk(KERN_DEBUG "%s: skipping word 93 validity check\n");
>         ide_dma_verbose()
>                 printk(", UDMA(66)"

> Please note that as a result ide-{cd,disk} device drivers won't report the
> DMA speed used but this is intended since now DMA mode being used is always
> reported by IDE core code.

> v2:
> * fixes suggested by Randy:
>   - use KERN_CONT for printk()-s in ide-{cd,disk}.c
>   - don't remove argument name from ide_xfer_verbose() declaration

> Cc: Nick Warne <nick@ukfsn.org>
> Cc: Mark Lord <lkml@rtr.ca>
> Cc: Randy Dunlap <randy.dunlap@oracle.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

[...]

> Index: b/drivers/ide/ide-dma.c
> ===================================================================
> --- a/drivers/ide/ide-dma.c
> +++ b/drivers/ide/ide-dma.c
> @@ -806,58 +809,26 @@ static int ide_dma_check(ide_drive_t *dr
>  	return vdma ? 0 : -1;
>  }
>  
> -void ide_dma_verbose(ide_drive_t *drive)
> +int ide_id_dma_bug(ide_drive_t *drive)
>  {
> -	struct hd_driveid *id	= drive->id;
> -	ide_hwif_t *hwif	= HWIF(drive);
> +	struct hd_driveid *id = drive->id;
>  
>  	if (id->field_valid & 4) {
>  		if ((id->dma_ultra >> 8) && (id->dma_mword >> 8))
[...]
> +			goto err_out;
>  	} else if (id->field_valid & 2) {
>  		if ((id->dma_mword >> 8) && (id->dma_1word >> 8))
> -			goto bug_dma_off;
> -		printk(", DMA");
> +			goto err_out;
>  	} else if (id->field_valid & 1) {

    Hm, bit 0 only gurantees that current translation

> -		goto bug_dma_off;
> +		if (id->tDMA == 0)

    Despite the name, this is not a transfer period but SW DMA mode number, so 
why mode 0 is bad?

> +			goto err_out;
>  	}
> -	return;
> -bug_dma_off:
> -	printk(", BUG DMA OFF");
> -	hwif->dma_off_quietly(drive);
> -	return;
> +	return 0;
> +err_out:
> +	printk(KERN_ERR "%s: bad DMA info in identify block\n", drive->name);
> +	return 1;
>  }
>  
> Index: b/drivers/ide/ide-lib.c
> ===================================================================
> --- a/drivers/ide/ide-lib.c
> +++ b/drivers/ide/ide-lib.c
> @@ -29,41 +29,44 @@
>   *	Add common non I/O op stuff here. Make sure it has proper
>   *	kernel-doc function headers or your patch will be rejected
>   */
> - 
> +
> +static const char *udma_str[] =
> +	 { "UDMA/16", "UDMA/25",  "UDMA/33",  "UDMA/44",
> +	   "UDMA/66", "UDMA/100", "UDMA/133", "UDMA7" };
> +static const char *mwdma_str[] =
> +	{ "MWDMA0", "MWDMA1", "MWDMA2" };
> +static const char *swdma_str[] =
> +	{ "SWDMA0", "SWDMA1", "SWDMA2" };
> +static const char *pio_str[] =
> +	{ "PIO0", "PIO1", "PIO2", "PIO3", "PIO4", "PIO5" };
>  
>  /**
>   *	ide_xfer_verbose	-	return IDE mode names
> - *	@xfer_rate: rate to name
> + *	@mode: transfer mode
>   *
>   *	Returns a constant string giving the name of the mode
>   *	requested.
>   */
>  
> -char *ide_xfer_verbose (u8 xfer_rate)
> +const char *ide_xfer_verbose(u8 mode)
>  {
[...]
> +	const char *s;
> +	u8 i = mode & 0xf;
> +
> +	if (mode >= XFER_UDMA_0 && mode <= XFER_UDMA_7)
> +		s = udma_str[i];
> +	else if (mode >= XFER_MW_DMA_0 && mode <= XFER_MW_DMA_2)
> +		s = mwdma_str[i];
> +	else if (mode >= XFER_SW_DMA_0 && mode <= XFER_SW_DMA_2)
> +		s = swdma_str[i];
> +	else if (mode >= XFER_PIO_0 && mode <= XFER_PIO_5)
> +		s = pio_str[i & 0x7];
> +	else if (mode == XFER_PIO_SLOW)
> +		s = "XFER SLOW";

    Not "PIO SLOW"?

> +	else
> +		s = "XFER ERROR";
> +
> +	return s;
>  }

MBR, Sergei

Re: Peculiar out-of-sync boot log lines

[Bartlomiej Zolnierkiewicz]

Hi,

On Friday 07 December 2007, Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
> > [PATCH] ide: DMA reporting and validity checking fixes (take 2)
> 
> > * ide_xfer_verbose() fixups:
> >   - beautify returned mode names
> >   - fix PIO5 reporting
> >   - make it return 'const char *'
> 
> > * Change printk() level from KERN_DEBUG to KERN_INFO in ide_find_dma_mode().
> 
> > * Add ide_id_dma_bug() helper based on ide_dma_verbose() to check for invalid
> >   DMA info in identify block.
> 
> > * Use ide_id_dma_bug() in ide_tune_dma() and ide_driveid_update().
> 
> >   As a result DMA won't be tuned or will be disabled after tuning if device
> >   reports inconsistent info about enabled DMA mode (ide_dma_verbose() does the
> >   same checks while the IDE device is probed by ide-{cd,disk} device driver).
> 
> > * Since (id->capability & 1) && id->tDMA is a valid configuration handle
> >   it correctly in ide_id_dma_bug().
> 
>     Huh? You don't check (id->capability & 1) there...
> 
> > * Remove no longer needed ide_dma_verbose().
> 
> > This patch should fix the following problem with out-of-sync IDE messages
> > reported by Nick Warne:
> 
> >        hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache<7>hdd:
> >        skipping word 93 validity check
> >         , UDMA(66)
> 
> > and later debugged by Mark Lord to be caused by:
> 
> >         ide_dma_verbose()
> >                 printk( ... "2048kB Cache");
> >         eighty_ninty_three()
> >                 printk(KERN_DEBUG "%s: skipping word 93 validity check\n");
> >         ide_dma_verbose()
> >                 printk(", UDMA(66)"
> 
> > Please note that as a result ide-{cd,disk} device drivers won't report the
> > DMA speed used but this is intended since now DMA mode being used is always
> > reported by IDE core code.
> 
> > v2:
> > * fixes suggested by Randy:
> >   - use KERN_CONT for printk()-s in ide-{cd,disk}.c
> >   - don't remove argument name from ide_xfer_verbose() declaration
> 
> > Cc: Nick Warne <nick@ukfsn.org>
> > Cc: Mark Lord <lkml@rtr.ca>
> > Cc: Randy Dunlap <randy.dunlap@oracle.com>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> 
> [...]
> 
> > Index: b/drivers/ide/ide-dma.c
> > ===================================================================
> > --- a/drivers/ide/ide-dma.c
> > +++ b/drivers/ide/ide-dma.c
> > @@ -806,58 +809,26 @@ static int ide_dma_check(ide_drive_t *dr
> >  	return vdma ? 0 : -1;
> >  }
> >  
> > -void ide_dma_verbose(ide_drive_t *drive)
> > +int ide_id_dma_bug(ide_drive_t *drive)
> >  {
> > -	struct hd_driveid *id	= drive->id;
> > -	ide_hwif_t *hwif	= HWIF(drive);
> > +	struct hd_driveid *id = drive->id;
> >  
> >  	if (id->field_valid & 4) {
> >  		if ((id->dma_ultra >> 8) && (id->dma_mword >> 8))
> [...]
> > +			goto err_out;
> >  	} else if (id->field_valid & 2) {
> >  		if ((id->dma_mword >> 8) && (id->dma_1word >> 8))
> > -			goto bug_dma_off;
> > -		printk(", DMA");
> > +			goto err_out;
> >  	} else if (id->field_valid & 1) {
> 
>     Hm, bit 0 only gurantees that current translation
> 
> > -		goto bug_dma_off;
> > +		if (id->tDMA == 0)
> 
>     Despite the name, this is not a transfer period but SW DMA mode number, so 
> why mode 0 is bad?

Thanks for checking the patch, I must have been half asleep while coding
this part cause it is completely bogus!

[ Well, it is not like that the code was OK before my changes... ;-) ]

I just removed incorrect (id->field_valid & 1) check in 'take 3'.

> > +			goto err_out;
> >  	}
> > -	return;
> > -bug_dma_off:
> > -	printk(", BUG DMA OFF");
> > -	hwif->dma_off_quietly(drive);
> > -	return;
> > +	return 0;
> > +err_out:
> > +	printk(KERN_ERR "%s: bad DMA info in identify block\n", drive->name);
> > +	return 1;
> >  }
> >  
> > Index: b/drivers/ide/ide-lib.c
> > ===================================================================
> > --- a/drivers/ide/ide-lib.c
> > +++ b/drivers/ide/ide-lib.c
> > @@ -29,41 +29,44 @@
> >   *	Add common non I/O op stuff here. Make sure it has proper
> >   *	kernel-doc function headers or your patch will be rejected
> >   */
> > - 
> > +
> > +static const char *udma_str[] =
> > +	 { "UDMA/16", "UDMA/25",  "UDMA/33",  "UDMA/44",
> > +	   "UDMA/66", "UDMA/100", "UDMA/133", "UDMA7" };
> > +static const char *mwdma_str[] =
> > +	{ "MWDMA0", "MWDMA1", "MWDMA2" };
> > +static const char *swdma_str[] =
> > +	{ "SWDMA0", "SWDMA1", "SWDMA2" };
> > +static const char *pio_str[] =
> > +	{ "PIO0", "PIO1", "PIO2", "PIO3", "PIO4", "PIO5" };
> >  
> >  /**
> >   *	ide_xfer_verbose	-	return IDE mode names
> > - *	@xfer_rate: rate to name
> > + *	@mode: transfer mode
> >   *
> >   *	Returns a constant string giving the name of the mode
> >   *	requested.
> >   */
> >  
> > -char *ide_xfer_verbose (u8 xfer_rate)
> > +const char *ide_xfer_verbose(u8 mode)
> >  {
> [...]
> > +	const char *s;
> > +	u8 i = mode & 0xf;
> > +
> > +	if (mode >= XFER_UDMA_0 && mode <= XFER_UDMA_7)
> > +		s = udma_str[i];
> > +	else if (mode >= XFER_MW_DMA_0 && mode <= XFER_MW_DMA_2)
> > +		s = mwdma_str[i];
> > +	else if (mode >= XFER_SW_DMA_0 && mode <= XFER_SW_DMA_2)
> > +		s = swdma_str[i];
> > +	else if (mode >= XFER_PIO_0 && mode <= XFER_PIO_5)
> > +		s = pio_str[i & 0x7];
> > +	else if (mode == XFER_PIO_SLOW)
> > +		s = "XFER SLOW";
> 
>     Not "PIO SLOW"?

fixed

While going through the patch I've noticed that ide_find_dma_mode() could
report the wrong mode for user requested and "CRC slow-down" speed changes,
this is also fixed now.

[PATCH] ide: DMA reporting and validity checking fixes (take 3)

* ide_xfer_verbose() fixups:
  - beautify returned mode names
  - fix PIO5 reporting
  - make it return 'const char *'

* Change printk() level from KERN_DEBUG to KERN_INFO in ide_find_dma_mode().

* Add ide_id_dma_bug() helper based on ide_dma_verbose() to check for invalid
  DMA info in identify block.

* Use ide_id_dma_bug() in ide_tune_dma() and ide_driveid_update().

  As a result DMA won't be tuned or will be disabled after tuning if device
  reports inconsistent info about enabled DMA mode (ide_dma_verbose() does the
  same checks while the IDE device is probed by ide-{cd,disk} device driver).

* Remove no longer needed ide_dma_verbose().

This patch should fix the following problem with out-of-sync IDE messages
reported by Nick Warne:

       hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB Cache<7>hdd:
       skipping word 93 validity check
        , UDMA(66)

and later debugged by Mark Lord to be caused by:

        ide_dma_verbose()
                printk( ... "2048kB Cache");
        eighty_ninty_three()
                printk(KERN_DEBUG "%s: skipping word 93 validity check\n");
        ide_dma_verbose()
                printk(", UDMA(66)"

Please note that as a result ide-{cd,disk} device drivers won't report the
DMA speed used but this is intended since now DMA mode being used is always
reported by IDE core code.

v2:
* fixes suggested by Randy:
  - use KERN_CONT for printk()-s in ide-{cd,disk}.c
  - don't remove argument name from ide_xfer_verbose() declaration

v3:
* Remove incorrect check for (id->field_valid & 1) from ide_id_dma_bug()
  (spotted by Sergei).

* "XFER SLOW" -> "PIO SLOW" in ide_xfer_verbose() (suggested by Sergei).

* Fix ide_find_dma_mode() to report the correct mode ('mode' after being
  limited by 'req_mode').

Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Cc: Nick Warne <nick@ukfsn.org>
Cc: Mark Lord <lkml@rtr.ca>
Cc: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-cd.c   |    7 -----
 drivers/ide/ide-disk.c |    7 +----
 drivers/ide/ide-dma.c  |   60 ++++++++++++-------------------------------------
 drivers/ide/ide-iops.c |    3 ++
 drivers/ide/ide-lib.c  |   55 +++++++++++++++++++++++---------------------
 include/linux/ide.h    |    6 ++--
 6 files changed, 53 insertions(+), 85 deletions(-)

Index: b/drivers/ide/ide-cd.c
===================================================================
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -3049,12 +3049,7 @@ int ide_cdrom_probe_capabilities (ide_dr
         else 	
         	printk(" drive");
 
-	printk(", %dkB Cache", be16_to_cpu(cap.buffer_size));
-
-	if (drive->using_dma)
-		ide_dma_verbose(drive);
-
-	printk("\n");
+	printk(KERN_CONT ", %dkB Cache\n", be16_to_cpu(cap.buffer_size));
 
 	return nslots;
 }
Index: b/drivers/ide/ide-disk.c
===================================================================
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -961,11 +961,8 @@ static void idedisk_setup (ide_drive_t *
 	if (id->buf_size)
 		printk (" w/%dKiB Cache", id->buf_size/2);
 
-	printk(", CHS=%d/%d/%d", 
-	       drive->bios_cyl, drive->bios_head, drive->bios_sect);
-	if (drive->using_dma)
-		ide_dma_verbose(drive);
-	printk("\n");
+	printk(KERN_CONT ", CHS=%d/%d/%d\n",
+			 drive->bios_cyl, drive->bios_head, drive->bios_sect);
 
 	/* write cache enabled? */
 	if ((id->csfo & 1) || (id->cfs_enable_1 & (1 << 5)))
Index: b/drivers/ide/ide-dma.c
===================================================================
--- a/drivers/ide/ide-dma.c
+++ b/drivers/ide/ide-dma.c
@@ -753,10 +753,12 @@ u8 ide_find_dma_mode(ide_drive_t *drive,
 			mode = XFER_MW_DMA_1;
 	}
 
-	printk(KERN_DEBUG "%s: %s mode selected\n", drive->name,
+	mode = min(mode, req_mode);
+
+	printk(KERN_INFO "%s: %s mode selected\n", drive->name,
 			  mode ? ide_xfer_verbose(mode) : "no DMA");
 
-	return min(mode, req_mode);
+	return mode;
 }
 
 EXPORT_SYMBOL_GPL(ide_find_dma_mode);
@@ -772,6 +774,9 @@ static int ide_tune_dma(ide_drive_t *dri
 	if (__ide_dma_bad_drive(drive))
 		return 0;
 
+	if (ide_id_dma_bug(drive))
+		return 0;
+
 	if (drive->hwif->host_flags & IDE_HFLAG_TRUST_BIOS_FOR_DMA)
 		return config_drive_for_dma(drive);
 
@@ -806,58 +811,23 @@ static int ide_dma_check(ide_drive_t *dr
 	return vdma ? 0 : -1;
 }
 
-void ide_dma_verbose(ide_drive_t *drive)
+int ide_id_dma_bug(ide_drive_t *drive)
 {
-	struct hd_driveid *id	= drive->id;
-	ide_hwif_t *hwif	= HWIF(drive);
+	struct hd_driveid *id = drive->id;
 
 	if (id->field_valid & 4) {
 		if ((id->dma_ultra >> 8) && (id->dma_mword >> 8))
-			goto bug_dma_off;
-		if (id->dma_ultra & ((id->dma_ultra >> 8) & hwif->ultra_mask)) {
-			if (((id->dma_ultra >> 11) & 0x1F) &&
-			    eighty_ninty_three(drive)) {
-				if ((id->dma_ultra >> 15) & 1) {
-					printk(", UDMA(mode 7)");
-				} else if ((id->dma_ultra >> 14) & 1) {
-					printk(", UDMA(133)");
-				} else if ((id->dma_ultra >> 13) & 1) {
-					printk(", UDMA(100)");
-				} else if ((id->dma_ultra >> 12) & 1) {
-					printk(", UDMA(66)");
-				} else if ((id->dma_ultra >> 11) & 1) {
-					printk(", UDMA(44)");
-				} else
-					goto mode_two;
-			} else {
-		mode_two:
-				if ((id->dma_ultra >> 10) & 1) {
-					printk(", UDMA(33)");
-				} else if ((id->dma_ultra >> 9) & 1) {
-					printk(", UDMA(25)");
-				} else if ((id->dma_ultra >> 8) & 1) {
-					printk(", UDMA(16)");
-				}
-			}
-		} else {
-			printk(", (U)DMA");	/* Can be BIOS-enabled! */
-		}
+			goto err_out;
 	} else if (id->field_valid & 2) {
 		if ((id->dma_mword >> 8) && (id->dma_1word >> 8))
-			goto bug_dma_off;
-		printk(", DMA");
-	} else if (id->field_valid & 1) {
-		goto bug_dma_off;
+			goto err_out;
 	}
-	return;
-bug_dma_off:
-	printk(", BUG DMA OFF");
-	hwif->dma_off_quietly(drive);
-	return;
+	return 0;
+err_out:
+	printk(KERN_ERR "%s: bad DMA info in identify block\n", drive->name);
+	return 1;
 }
 
-EXPORT_SYMBOL(ide_dma_verbose);
-
 int ide_set_dma(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;
Index: b/drivers/ide/ide-iops.c
===================================================================
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -748,6 +748,9 @@ int ide_driveid_update(ide_drive_t *driv
 		drive->id->dma_1word = id->dma_1word;
 		/* anything more ? */
 		kfree(id);
+
+		if (drive->using_dma && ide_id_dma_bug(drive))
+			ide_dma_off(drive);
 	}
 
 	return 1;
Index: b/drivers/ide/ide-lib.c
===================================================================
--- a/drivers/ide/ide-lib.c
+++ b/drivers/ide/ide-lib.c
@@ -29,41 +29,44 @@
  *	Add common non I/O op stuff here. Make sure it has proper
  *	kernel-doc function headers or your patch will be rejected
  */
- 
+
+static const char *udma_str[] =
+	 { "UDMA/16", "UDMA/25",  "UDMA/33",  "UDMA/44",
+	   "UDMA/66", "UDMA/100", "UDMA/133", "UDMA7" };
+static const char *mwdma_str[] =
+	{ "MWDMA0", "MWDMA1", "MWDMA2" };
+static const char *swdma_str[] =
+	{ "SWDMA0", "SWDMA1", "SWDMA2" };
+static const char *pio_str[] =
+	{ "PIO0", "PIO1", "PIO2", "PIO3", "PIO4", "PIO5" };
 
 /**
  *	ide_xfer_verbose	-	return IDE mode names
- *	@xfer_rate: rate to name
+ *	@mode: transfer mode
  *
  *	Returns a constant string giving the name of the mode
  *	requested.
  */
 
-char *ide_xfer_verbose (u8 xfer_rate)
+const char *ide_xfer_verbose(u8 mode)
 {
-        switch(xfer_rate) {
-                case XFER_UDMA_7:	return("UDMA 7");
-                case XFER_UDMA_6:	return("UDMA 6");
-                case XFER_UDMA_5:	return("UDMA 5");
-                case XFER_UDMA_4:	return("UDMA 4");
-                case XFER_UDMA_3:	return("UDMA 3");
-                case XFER_UDMA_2:	return("UDMA 2");
-                case XFER_UDMA_1:	return("UDMA 1");
-                case XFER_UDMA_0:	return("UDMA 0");
-                case XFER_MW_DMA_2:	return("MW DMA 2");
-                case XFER_MW_DMA_1:	return("MW DMA 1");
-                case XFER_MW_DMA_0:	return("MW DMA 0");
-                case XFER_SW_DMA_2:	return("SW DMA 2");
-                case XFER_SW_DMA_1:	return("SW DMA 1");
-                case XFER_SW_DMA_0:	return("SW DMA 0");
-                case XFER_PIO_4:	return("PIO 4");
-                case XFER_PIO_3:	return("PIO 3");
-                case XFER_PIO_2:	return("PIO 2");
-                case XFER_PIO_1:	return("PIO 1");
-                case XFER_PIO_0:	return("PIO 0");
-                case XFER_PIO_SLOW:	return("PIO SLOW");
-                default:		return("XFER ERROR");
-        }
+	const char *s;
+	u8 i = mode & 0xf;
+
+	if (mode >= XFER_UDMA_0 && mode <= XFER_UDMA_7)
+		s = udma_str[i];
+	else if (mode >= XFER_MW_DMA_0 && mode <= XFER_MW_DMA_2)
+		s = mwdma_str[i];
+	else if (mode >= XFER_SW_DMA_0 && mode <= XFER_SW_DMA_2)
+		s = swdma_str[i];
+	else if (mode >= XFER_PIO_0 && mode <= XFER_PIO_5)
+		s = pio_str[i & 0x7];
+	else if (mode == XFER_PIO_SLOW)
+		s = "PIO SLOW";
+	else
+		s = "XFER ERROR";
+
+	return s;
 }
 
 EXPORT_SYMBOL(ide_xfer_verbose);
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1255,6 +1255,7 @@ int ide_in_drive_list(struct hd_driveid 
 
 #ifdef CONFIG_BLK_DEV_IDEDMA
 int __ide_dma_bad_drive(ide_drive_t *);
+int ide_id_dma_bug(ide_drive_t *);
 
 u8 ide_find_dma_mode(ide_drive_t *, u8);
 
@@ -1264,7 +1265,6 @@ static inline u8 ide_max_dma_mode(ide_dr
 }
 
 void ide_dma_off(ide_drive_t *);
-void ide_dma_verbose(ide_drive_t *);
 int ide_set_dma(ide_drive_t *);
 ide_startstop_t ide_dma_intr(ide_drive_t *);
 
@@ -1287,6 +1287,7 @@ extern void ide_dma_timeout(ide_drive_t 
 #endif /* CONFIG_BLK_DEV_IDEDMA_PCI */
 
 #else
+static inline int ide_id_dma_bug(ide_drive_t *drive) { return 0; }
 static inline u8 ide_find_dma_mode(ide_drive_t *drive, u8 speed) { return 0; }
 static inline u8 ide_max_dma_mode(ide_drive_t *drive) { return 0; }
 static inline void ide_dma_off(ide_drive_t *drive) { ; }
@@ -1333,8 +1334,7 @@ static inline void ide_set_hwifdata (ide
 	hwif->hwif_data = data;
 }
 
-/* ide-lib.c */
-extern char *ide_xfer_verbose(u8 xfer_rate);
+const char *ide_xfer_verbose(u8 mode);
 extern void ide_toggle_bounce(ide_drive_t *drive, int on);
 extern int ide_set_xfer_rate(ide_drive_t *drive, u8 rate);
 

Re: Peculiar out-of-sync boot log lines

[Nick Warne]

On Sun, 2 Dec 2007 19:30:34 +0100
Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:

Hi Bart,

Top posting! grrrr.

This patch works fine on my system with this peculiar DVD drive, and
log reports are perfect.

Updated to Linus' git today - 2.6.24-rc6-g5b825ed2

/var/log/messages:

Dec 23 09:36:04 linuxamd kernel:     ide0: BM-DMA at 0xd000-0xd007, BIOS settings: hda:DMA, hdb:DMA
Dec 23 09:36:04 linuxamd kernel:     ide1: BM-DMA at 0xd008-0xd00f, BIOS settings: hdc:DMA, hdd:DMA
Dec 23 09:36:04 linuxamd kernel: hda: UDMA/66 mode selected
Dec 23 09:36:04 linuxamd kernel: hdb: UDMA/66 mode selected
Dec 23 09:36:04 linuxamd kernel: hdc: UDMA/66 mode selected
Dec 23 09:36:04 linuxamd kernel: hdd: UDMA/66 mode selected
Dec 23 09:36:04 linuxamd kernel: hda: max request size: 128KiB
Dec 23 09:36:04 linuxamd kernel: hda: 160086528 sectors (81964 MB) w/2048KiB Cache, CHS=65535/16/63
Dec 23 09:36:04 linuxamd kernel: hda: cache flushes supported
Dec 23 09:36:04 linuxamd kernel:  hda: hda1 hda2 hda3 hda4
Dec 23 09:36:04 linuxamd kernel: hdb: max request size: 128KiB
Dec 23 09:36:04 linuxamd kernel: hdb: 30015216 sectors (15367 MB) w/2048KiB Cache, CHS=29777/16/63
Dec 23 09:36:04 linuxamd kernel: hdb: cache flushes not supported
Dec 23 09:36:04 linuxamd kernel:  hdb: hdb1
Dec 23 09:36:04 linuxamd kernel: hdc: max request size: 128KiB
Dec 23 09:36:04 linuxamd kernel: hdc: 39876480 sectors (20416 MB) w/2048KiB Cache, CHS=39560/16/63
Dec 23 09:36:04 linuxamd kernel: hdc: cache flushes not supported
Dec 23 09:36:04 linuxamd kernel:  hdc: hdc1
Dec 23 09:36:04 linuxamd kernel: hdd: ATAPI 48X DVD-ROM DVD-R-RAM
CD-R/RW drive, 2048kB Cache

and

/var/log/syslog

Dec 23 09:36:04 linuxamd kernel: hdb: Maxtor 51536H2, ATA DISK drive
Dec 23 09:36:04 linuxamd kernel: hda: Maxtor 6Y080L0, ATA DISK drive
Dec 23 09:36:04 linuxamd kernel: hdd: TSSTcorp CDDVDW SH-S202J, ATAPI CD/DVD-ROM drive
Dec 23 09:36:04 linuxamd kernel: hdc: Maxtor 52049H3, ATA DISK drive


> On Sunday 02 December 2007, Randy Dunlap wrote:
> > On Sat, 1 Dec 2007 22:59:35 +0100 Bartlomiej Zolnierkiewicz wrote:
> > 
> > > Thanks for reporting/debugging it guys!
> > > 
> > > > Something in there needs to insert a '\n' before the "skipping
> > > > word" message. Since it doesn't do that right now, the
> > > > KERN_DEBUG string appears as "<7>"
> > > 
> > > This seems like a good occasion to fix ide_dma_verbose() for good
> > > so... :)
> > > 
> > > [ patch is against current Linus tree so might not apply to
> > > 2.6.23.9 ]
> > > 
> > > [PATCH] ide: DMA reporting and validity checking fixes
> > > 
> > > * ide_xfer_verbose() fixups:
> > >   - beautify returned mode names
> > >   - fix PIO5 reporting
> > >   - make it return 'const char *'
> > > 
> > > * Change printk() level from KERN_DEBUG to KERN_INFO in
> > > ide_find_dma_mode().
> > > 
> > > * Add ide_id_dma_bug() helper based on ide_dma_verbose() to check
> > > for invalid DMA info in identify block.
> > > 
> > > * Use ide_id_dma_bug() in ide_tune_dma() and ide_driveid_update().
> > > 
> > >   As a result DMA won't be tuned or will be disabled after tuning
> > > if device reports inconsistent info about enabled DMA mode
> > > (ide_dma_verbose() does the same checks while the IDE device is
> > > probed by ide-{cd,disk} device driver).
> > > 
> > > * Since (id->capability & 1) && id->tDMA is a valid configuration
> > > handle it correctly in ide_id_dma_bug().
> > > 
> > > * Remove no longer needed ide_dma_verbose().
> > > 
> > > This patch should fix the following problem with out-of-sync IDE
> > > messages reported by Nick Warned:
> > > 
> > >        hdd: ATAPI 48X DVD-ROM DVD-R-RAM CD-R/RW drive, 2048kB
> > > Cache<7>hdd: skipping word 93 validity check
> > >         , UDMA(66)
> > > 
> > > and later debugged by Mark Lord to be caused by:
> > > 
> > >         ide_dma_verbose()
> > >                 printk( ... "2048kB Cache");
> > >         eighty_ninty_three()
> > >                 printk(KERN_DEBUG "%s: skipping word 93 validity
> > > check\n"); ide_dma_verbose()
> > >                 printk(", UDMA(66)"
> > > 
> > > Please note that as a result ide-{cd,disk} device drivers won't
> > > report the DMA speed used but this is intended since now DMA mode
> > > being used is always reported by IDE core code.
> > > 
> > > Cc: Nick Warne <nick@ukfsn.org>
> > > Cc: Mark Lord <lkml@rtr.ca>
> > > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

~ skip

Nick
-- 
Free Software Foundation Associate Member 5508