linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* PATCH: IDE - do spin up for all platforms
@ 2004-08-09 14:39 Alan Cox
  2004-08-09 14:57 ` Jeff Garzik
  0 siblings, 1 reply; 4+ messages in thread
From: Alan Cox @ 2004-08-09 14:39 UTC (permalink / raw)
  To: linux-ide, akpm

This was put in for PPC specifically and defined in the 2.4 tree to be
paranoid about breaking stuff. The actual wait for hwif spin up is however
perfectly correct for all drives. In the normal PC case the PC BIOS has done
this but with other embedded boxes and with things like LinuxBIOS it may not
have done. Thus it should always be done.

--- drivers/ide/ide-probe.c~	2004-08-09 15:36:13.109053400 +0100
+++ drivers/ide/ide-probe.c	2004-08-09 15:36:13.109053400 +0100
@@ -635,7 +635,6 @@
 	device_register(&hwif->gendev);
 }
 
-#ifdef CONFIG_PPC
 static int wait_hwif_ready(ide_hwif_t *hwif)
 {
 	int rc;
@@ -671,7 +670,6 @@
 	
 	return rc;
 }
-#endif
 
 /*
  * This routine only knows how to look for drive units 0 and 1
@@ -717,7 +715,6 @@
 
 	local_irq_set(flags);
 
-#ifdef CONFIG_PPC
 	/* This is needed on some PPCs and a bunch of BIOS-less embedded
 	 * platforms. Typical cases are:
 	 * 
@@ -739,7 +736,6 @@
 	 */
 	if (wait_hwif_ready(hwif))
 		printk(KERN_WARNING "%s: Wait for ready failed before probe !\n", hwif->name);
-#endif /* CONFIG_PPC */
 
 	/*
 	 * Second drive should only exist if first drive was found,

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

* Re: PATCH: IDE - do spin up for all platforms
  2004-08-09 14:39 PATCH: IDE - do spin up for all platforms Alan Cox
@ 2004-08-09 14:57 ` Jeff Garzik
  2004-08-09 15:25   ` Alan Cox
  2004-08-09 22:10   ` Benjamin Herrenschmidt
  0 siblings, 2 replies; 4+ messages in thread
From: Jeff Garzik @ 2004-08-09 14:57 UTC (permalink / raw)
  To: Alan Cox; +Cc: linux-ide, akpm

Alan Cox wrote:
> This was put in for PPC specifically and defined in the 2.4 tree to be
> paranoid about breaking stuff. The actual wait for hwif spin up is however
> perfectly correct for all drives. In the normal PC case the PC BIOS has done
> this but with other embedded boxes and with things like LinuxBIOS it may not
> have done. Thus it should always be done.
> 
> --- drivers/ide/ide-probe.c~	2004-08-09 15:36:13.109053400 +0100
> +++ drivers/ide/ide-probe.c	2004-08-09 15:36:13.109053400 +0100
> @@ -635,7 +635,6 @@
>  	device_register(&hwif->gendev);
>  }
>  
> -#ifdef CONFIG_PPC
>  static int wait_hwif_ready(ide_hwif_t *hwif)
>  {
>  	int rc;
> @@ -671,7 +670,6 @@
>  	
>  	return rc;
>  }
> -#endif
>  
>  /*
>   * This routine only knows how to look for drive units 0 and 1
> @@ -717,7 +715,6 @@
>  
>  	local_irq_set(flags);
>  
> -#ifdef CONFIG_PPC
>  	/* This is needed on some PPCs and a bunch of BIOS-less embedded
>  	 * platforms. Typical cases are:
>  	 * 
> @@ -739,7 +736,6 @@
>  	 */
>  	if (wait_hwif_ready(hwif))
>  		printk(KERN_WARNING "%s: Wait for ready failed before probe !\n", hwif->name);
> -#endif /* CONFIG_PPC */


It needs to check for "status != 0x7f" which is the standard "no device 
here" return status.

But worse than that, status can return 0xFF for "no hardware there", in 
which case this routine will hang the boot for 35 seconds.

Note also that libata code does this:
* wait 7 seconds
* printk("it's taking a long time, be patient")
* wait ~30 seconds

	Jeff



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

* Re: PATCH: IDE - do spin up for all platforms
  2004-08-09 14:57 ` Jeff Garzik
@ 2004-08-09 15:25   ` Alan Cox
  2004-08-09 22:10   ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 4+ messages in thread
From: Alan Cox @ 2004-08-09 15:25 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Alan Cox, linux-ide, akpm

On Mon, Aug 09, 2004 at 10:57:34AM -0400, Jeff Garzik wrote:
> It needs to check for "status != 0x7f" which is the standard "no device 
> here" return status.

It checks for BUSY_STAT

> But worse than that, status can return 0xFF for "no hardware there", in 
> which case this routine will hang the boot for 35 seconds.

It already handles the case where 0xFF is returned, has done for years
because people get this wrong and also because hardware going missing tends
to return 0xFF (eg PCMCIA cards).

So the code already does everything you comment on

0x7F will return from the wait (BUSY_STAT clear)
0xFF will return with an error



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

* Re: PATCH: IDE - do spin up for all platforms
  2004-08-09 14:57 ` Jeff Garzik
  2004-08-09 15:25   ` Alan Cox
@ 2004-08-09 22:10   ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2004-08-09 22:10 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Alan Cox, linux-ide, Andrew Morton

On Tue, 2004-08-10 at 00:57, Jeff Garzik wrote:

> It needs to check for "status != 0x7f" which is the standard "no device 
> here" return status.

Nope, it waits for BSY to go down

> But worse than that, status can return 0xFF for "no hardware there", in 
> which case this routine will hang the boot for 35 seconds.

My original version did, let me check.. yes, the current code does that
too, check ide_wait_not_busy. Note that this is a workaround for broken
hardware as the spec clearly says that D7 should be pulled low so that
no drive returns 0x7f (BSY clear). However, there is a significant
amount of HW with this bug...

> Note also that libata code does this:
> * wait 7 seconds
> * printk("it's taking a long time, be patient")
> * wait ~30 seconds

That could be a good idea. Patch welcome :)

Ben.



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

end of thread, other threads:[~2004-08-09 22:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-08-09 14:39 PATCH: IDE - do spin up for all platforms Alan Cox
2004-08-09 14:57 ` Jeff Garzik
2004-08-09 15:25   ` Alan Cox
2004-08-09 22:10   ` Benjamin Herrenschmidt

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