[U-Boot] SATA/IDE/etc... init

[U-Boot] SATA/IDE/etc... init

[Mike Frysinger]

the SATA/IDE/etc... layers seem to go against the u-boot policy when it comes 
to initialization.  in other words, if support for them is enabled, then they 
are unconditionally initialized in board code (see ide_init / sata_initialize 
in lib_*/board.c:board_init_r).  am i missing something or should these init 
functions be delayed until the sata/ide/whatever functions need them ?  or 
should they be made explicit like some commands where you first have to do 
"sata init" before you can access the device ?

this bugs me most because normally u-boot starts up nice & fast, but now that 
we've added SATA support, there's an annoying second or 3 pause while the disk 
is probed ...
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081211/4845a4e5/attachment.pgp 

[U-Boot] SATA/IDE/etc... init

[Wolfgang Denk]

Dear Mike Frysinger,

In message <200812110304.12784.vapier@gentoo.org> you wrote:
>
> the SATA/IDE/etc... layers seem to go against the u-boot policy when it comes
> to initialization.  in other words, if support for them is enabled, then they
> are unconditionally initialized in board code (see ide_init / sata_initialize
> in lib_*/board.c:board_init_r).  am i missing something or should these init
> functions be delayed until the sata/ide/whatever functions need them ?  or

Yes, you are right.

Blame this on me - this was inherited from the first implementation of
PCMCIA support on the MPC8xx, where I found it useful to see the state
when the sytem booted. I learned that leasson only later... Sorry.

> should they be made explicit like some commands where you first have to do
> "sata init" before you can access the device ?

Correct.

> this bugs me most because normally u-boot starts up nice & fast, but now that
> we've added SATA support, there's an annoying second or 3 pause while the disk
> is probed ...

Patches appreciated :-)

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I dislike companies that have a we-are-the-high-priests-of-hardware-
so-you'll-like-what-we-give-you attitude. I like commodity markets in
which iron-and-silicon hawkers know that they exist to  provide  fast
toys for software types like me to play with..."    - Eric S. Raymond

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Mike Frysinger]

Rather than have the board code initialize SATA automatically during boot,
make the user manually run "sata init".  This brings the SATA subsystem in
line with common U-Boot policy.

Along these lines, the "is_sata_supported()" hook is no longer needed, so
scrub it from the tree.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 board/freescale/mpc8536ds/mpc8536ds.c |   11 -----------
 common/cmd_sata.c                     |    7 +++++--
 include/sata.h                        |    2 --
 lib_ppc/board.c                       |   20 --------------------
 4 files changed, 5 insertions(+), 35 deletions(-)

diff --git a/board/freescale/mpc8536ds/mpc8536ds.c b/board/freescale/mpc8536ds/mpc8536ds.c
index 2b17612..ada8020 100644
--- a/board/freescale/mpc8536ds/mpc8536ds.c
+++ b/board/freescale/mpc8536ds/mpc8536ds.c
@@ -582,17 +582,6 @@ get_board_ddr_clk(ulong dummy)
 }
 #endif
 
-int is_sata_supported(void)
-{
-	volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
-	uint sdrs2_io_sel =
-		(gur->pordevsr & MPC85xx_PORDEVSR_SRDS2_IO_SEL) >> 27;
-	if (sdrs2_io_sel & 0x04)
-		return 0;
-
-	return 1;
-}
-
 int board_eth_init(bd_t *bis)
 {
 #ifdef CONFIG_TSEC_ENET
diff --git a/common/cmd_sata.c b/common/cmd_sata.c
index dd6f1d9..7795f39 100644
--- a/common/cmd_sata.c
+++ b/common/cmd_sata.c
@@ -31,7 +31,7 @@
 int curr_device = -1;
 block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
 
-int sata_initialize(void)
+static int sata_initialize(void)
 {
 	int rc;
 	int i;
@@ -71,7 +71,9 @@ int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 		printf("Usage:\n%s\n", cmdtp->usage);
 		return 1;
 	case 2:
-		if (strncmp(argv[1],"inf", 3) == 0) {
+		if (strcmp(argv[1], "init") == 0) {
+			return sata_initialize();
+		} else if (strncmp(argv[1],"inf", 3) == 0) {
 			int i;
 			putc('\n');
 			for (i = 0; i < CONFIG_SYS_SATA_MAX_DEVICE; ++i) {
@@ -186,6 +188,7 @@ int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	sata, 5, 1, do_sata,
 	"sata	- SATA sub system\n",
+	"init - init SATA sub system\n"
 	"sata info - show available SATA devices\n"
 	"sata device [dev] - show or set current device\n"
 	"sata part [dev] - print partition table\n"
diff --git a/include/sata.h b/include/sata.h
index 57ee9ac..d07ca5a 100644
--- a/include/sata.h
+++ b/include/sata.h
@@ -6,6 +6,4 @@ int scan_sata(int dev);
 ulong sata_read(int dev, ulong blknr, ulong blkcnt, void *buffer);
 ulong sata_write(int dev, ulong blknr, ulong blkcnt, const void *buffer);
 
-int sata_initialize(void);
-
 #endif
diff --git a/lib_ppc/board.c b/lib_ppc/board.c
index 289a32a..3076824 100644
--- a/lib_ppc/board.c
+++ b/lib_ppc/board.c
@@ -38,9 +38,6 @@
 #if defined(CONFIG_CMD_IDE)
 #include <ide.h>
 #endif
-#if defined(CONFIG_CMD_SATA)
-#include <sata.h>
-#endif
 #if defined(CONFIG_CMD_SCSI)
 #include <scsi.h>
 #endif
@@ -635,16 +632,6 @@ void board_init_f (ulong bootflag)
 	/* NOTREACHED - relocate_code() does not return */
 }
 
-int __is_sata_supported(void)
-{
-	/* For some boards, when sata disabled by the switch, and the
-	 * driver still access the sata registers, the cpu will hangup.
-	 * please define platform specific is_sata_supported() if your
-	 * board have such issue.*/
-	return 1;
-}
-int is_sata_supported(void) __attribute__((weak, alias("__is_sata_supported")));
-
 /************************************************************************
  *
  * This is the next part if the initialization sequence: we are now
@@ -1144,13 +1131,6 @@ void board_init_r (gd_t *id, ulong dest_addr)
 #endif
 #endif
 
-#if defined(CONFIG_CMD_SATA)
-	if (is_sata_supported()) {
-		puts("SATA:  ");
-		sata_initialize();
-	}
-#endif
-
 #ifdef CONFIG_LAST_STAGE_INIT
 	WATCHDOG_RESET ();
 	/*
-- 
1.6.0.4

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Kumar Gala]

On Dec 11, 2008, at 4:32 AM, Mike Frysinger wrote:

> Rather than have the board code initialize SATA automatically during  
> boot,
> make the user manually run "sata init".  This brings the SATA  
> subsystem in
> line with common U-Boot policy.
>
> Along these lines, the "is_sata_supported()" hook is no longer  
> needed, so
> scrub it from the tree.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> board/freescale/mpc8536ds/mpc8536ds.c |   11 -----------
> common/cmd_sata.c                     |    7 +++++--
> include/sata.h                        |    2 --
> lib_ppc/board.c                       |   20 --------------------
> 4 files changed, 5 insertions(+), 35 deletions(-)

Why should we make the user have to do 'sata init'?  That seems  
annoying and easy to forget.

- k

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Mike Frysinger]

On Thursday 11 December 2008 11:17:56 Kumar Gala wrote:
> On Dec 11, 2008, at 4:32 AM, Mike Frysinger wrote:
> > Rather than have the board code initialize SATA automatically during
> > boot,
> > make the user manually run "sata init".  This brings the SATA
> > subsystem in
> > line with common U-Boot policy.
> >
> > Along these lines, the "is_sata_supported()" hook is no longer
> > needed, so
> > scrub it from the tree.
> >
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > ---
> > board/freescale/mpc8536ds/mpc8536ds.c |   11 -----------
> > common/cmd_sata.c                     |    7 +++++--
> > include/sata.h                        |    2 --
> > lib_ppc/board.c                       |   20 --------------------
> > 4 files changed, 5 insertions(+), 35 deletions(-)
>
> Why should we make the user have to do 'sata init'?  That seems
> annoying and easy to forget.

it's already standard behavior with most mass storage things (like mmc and sf 
and ...), so it isnt a big deal imo.  i asked if Wolfgang wanted it automated 
and he preferred making the user do it themselves.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081211/8a067e61/attachment.pgp 

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Kumar Gala]

On Dec 11, 2008, at 2:03 PM, Mike Frysinger wrote:

> On Thursday 11 December 2008 11:17:56 Kumar Gala wrote:
>> On Dec 11, 2008, at 4:32 AM, Mike Frysinger wrote:
>>> Rather than have the board code initialize SATA automatically during
>>> boot,
>>> make the user manually run "sata init".  This brings the SATA
>>> subsystem in
>>> line with common U-Boot policy.
>>>
>>> Along these lines, the "is_sata_supported()" hook is no longer
>>> needed, so
>>> scrub it from the tree.
>>>
>>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>>> ---
>>> board/freescale/mpc8536ds/mpc8536ds.c |   11 -----------
>>> common/cmd_sata.c                     |    7 +++++--
>>> include/sata.h                        |    2 --
>>> lib_ppc/board.c                       |   20 --------------------
>>> 4 files changed, 5 insertions(+), 35 deletions(-)
>>
>> Why should we make the user have to do 'sata init'?  That seems
>> annoying and easy to forget.
>
> it's already standard behavior with most mass storage things (like  
> mmc and sf
> and ...), so it isnt a big deal imo.  i asked if Wolfgang wanted it  
> automated
> and he preferred making the user do it themselves.

This seems backwards to me..

'sata init' isn't safe.  It seems like you should only be able to call  
it once.  However I think we can keep issuing it and cause bad things  
to happen.

Also, in the code you removed we do a runtime check on 8536 to see if  
SATA is even available.  That check is still valid.

- k

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Mike Frysinger]

On Thursday 11 December 2008 15:51:32 Kumar Gala wrote:
> On Dec 11, 2008, at 2:03 PM, Mike Frysinger wrote:
> > On Thursday 11 December 2008 11:17:56 Kumar Gala wrote:
> >> On Dec 11, 2008, at 4:32 AM, Mike Frysinger wrote:
> >>> Rather than have the board code initialize SATA automatically during
> >>> boot,
> >>> make the user manually run "sata init".  This brings the SATA
> >>> subsystem in
> >>> line with common U-Boot policy.
> >>>
> >>> Along these lines, the "is_sata_supported()" hook is no longer
> >>> needed, so
> >>> scrub it from the tree.
> >>>
> >>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> >>> ---
> >>> board/freescale/mpc8536ds/mpc8536ds.c |   11 -----------
> >>> common/cmd_sata.c                     |    7 +++++--
> >>> include/sata.h                        |    2 --
> >>> lib_ppc/board.c                       |   20 --------------------
> >>> 4 files changed, 5 insertions(+), 35 deletions(-)
> >>
> >> Why should we make the user have to do 'sata init'?  That seems
> >> annoying and easy to forget.
> >
> > it's already standard behavior with most mass storage things (like
> > mmc and sf and ...), so it isnt a big deal imo.  i asked if Wolfgang
> > wanted it automated and he preferred making the user do it themselves.
>
> This seems backwards to me..
>
> 'sata init' isn't safe.  It seems like you should only be able to call
> it once.  However I think we can keep issuing it and cause bad things
> to happen.

i dont think so.  the SATA driver should be doing the right thing: init_sata() 
should get the hardware into a usable state regardless of how it was before.

> Also, in the code you removed we do a runtime check on 8536 to see if
> SATA is even available.  That check is still valid.

why ?  if the hardware doesnt support it, then the user shouldnt be attempting 
to use it.  if they do, that's their fault for doing something stupid.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081211/43638103/attachment.pgp 

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Scott Wood]

Mike Frysinger wrote:
> On Thursday 11 December 2008 15:51:32 Kumar Gala wrote:
>> 'sata init' isn't safe.  It seems like you should only be able to call
>> it once.  However I think we can keep issuing it and cause bad things
>> to happen.
> 
> i dont think so.  the SATA driver should be doing the right thing: init_sata() 
> should get the hardware into a usable state regardless of how it was before.

So then what's the downside to having any sata access automatically call 
init_sata()?

>> Also, in the code you removed we do a runtime check on 8536 to see if
>> SATA is even available.  That check is still valid.
> 
> why ?  if the hardware doesnt support it, then the user shouldnt be attempting 
> to use it.  if they do, that's their fault for doing something stupid.

There's no need to be unnecessarily user-hostile.

-Scott

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Mike Frysinger]

On Thursday 11 December 2008 16:01:33 Scott Wood wrote:
> Mike Frysinger wrote:
> > On Thursday 11 December 2008 15:51:32 Kumar Gala wrote:
> >> 'sata init' isn't safe.  It seems like you should only be able to call
> >> it once.  However I think we can keep issuing it and cause bad things
> >> to happen.
> >
> > i dont think so.  the SATA driver should be doing the right thing:
> > init_sata() should get the hardware into a usable state regardless of how
> > it was before.
>
> So then what's the downside to having any sata access automatically call
> init_sata()?

i dont care either way.  read the earlier part where i said Wolfgang preferred 
this way.

having init_sata() be automatic though would prevent hotplugging ... but maybe 
people dont care about that ...

> >> Also, in the code you removed we do a runtime check on 8536 to see if
> >> SATA is even available.  That check is still valid.
> >
> > why ?  if the hardware doesnt support it, then the user shouldnt be
> > attempting to use it.  if they do, that's their fault for doing something
> > stupid.
>
> There's no need to be unnecessarily user-hostile.

there's no need to make u-boot unnecessarily safe for the people who dont 
understand the system
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081211/2d1be75c/attachment.pgp 

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Scott Wood]

Mike Frysinger wrote:
> On Thursday 11 December 2008 16:01:33 Scott Wood wrote:
>> Mike Frysinger wrote:
>>> why ?  if the hardware doesnt support it, then the user shouldnt be
>>> attempting to use it.  if they do, that's their fault for doing something
>>> stupid.
>> There's no need to be unnecessarily user-hostile.
> 
> there's no need to make u-boot unnecessarily safe for the people who dont 
> understand the system

Oh, please.  It's not like it's stopping any reasonable usage (quite the 
opposite -- removing the check removes a way for the user to find out 
whether SATA is present); it's just avoiding exposing a device that does 
not actually exist.

-Scott

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Mike Frysinger]

On Thursday 11 December 2008 16:16:12 Scott Wood wrote:
> Mike Frysinger wrote:
> > On Thursday 11 December 2008 16:01:33 Scott Wood wrote:
> >> Mike Frysinger wrote:
> >>> why ?  if the hardware doesnt support it, then the user shouldnt be
> >>> attempting to use it.  if they do, that's their fault for doing
> >>> something stupid.
> >>
> >> There's no need to be unnecessarily user-hostile.
> >
> > there's no need to make u-boot unnecessarily safe for the people who dont
> > understand the system
>
> Oh, please.  It's not like it's stopping any reasonable usage (quite the
> opposite -- removing the check removes a way for the user to find out
> whether SATA is present); it's just avoiding exposing a device that does
> not actually exist.

it made sense when the init step was automatic.  but you're telling me users 
of your board are incapable of looking at it and going "hmm, this has a SATA 
disk" ?  it isnt like disks are tiny and they have to scan a board for some 
obscure IC.  disks are friggin huge.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081211/ced4397f/attachment.pgp 

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Scott Wood]

Mike Frysinger wrote:
> it made sense when the init step was automatic.  but you're telling me users 
> of your board are incapable of looking at it and going "hmm, this has a SATA 
> disk" ?  it isnt like disks are tiny and they have to scan a board for some 
> obscure IC.  disks are friggin huge.

It doesn't matter how big it is, if it's in some remote board farm -- or 
if I'm considering adding a disk, and I want to see if the board is 
configured to use those pins for SATA or for something else (and thus 
whether I need to dig out the board manual and look at DIP switch 
configuration).

What harm does the check do?

-Scott

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Mike Frysinger]

On Thursday 11 December 2008 16:36:12 Scott Wood wrote:
> Mike Frysinger wrote:
> > it made sense when the init step was automatic.  but you're telling me
> > users of your board are incapable of looking at it and going "hmm, this
> > has a SATA disk" ?  it isnt like disks are tiny and they have to scan a
> > board for some obscure IC.  disks are friggin huge.
>
> It doesn't matter how big it is, if it's in some remote board farm -- or
> if I'm considering adding a disk, and I want to see if the board is
> configured to use those pins for SATA or for something else (and thus
> whether I need to dig out the board manual and look at DIP switch
> configuration).
>
> What harm does the check do?

size/runtime overhead for one board where the only thing gained is a slightly 
tweaked user experience (i.e. no functional difference)
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081211/e4eeba1c/attachment.pgp 

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Wolfgang Denk]

Dear Mike Frysinger,

In message <200812111624.20543.vapier@gentoo.org> you wrote:
>
> it made sense when the init step was automatic.  but you're telling me users 

And we still want to have it automatic, just deferred.

> of your board are incapable of looking at it and going "hmm, this has a SATA 
> disk" ?  it isnt like disks are tiny and they have to scan a board for some
> obscure IC.  disks are friggin huge.

Come on. Have you never seen a system that came in a case, and you
didn't know if it included a disk, or how many of these?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Our OS who art in CPU, UNIX be thy name.
Thy programs run, thy syscalls done,
In kernel as it is in user!

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Mike Frysinger]

On Thursday 11 December 2008 18:57:37 Wolfgang Denk wrote:
> In message <200812111624.20543.vapier@gentoo.org> you wrote:
> > it made sense when the init step was automatic.  but you're telling me
> > users
>
> And we still want to have it automatic, just deferred.
>
> > of your board are incapable of looking at it and going "hmm, this has a
> > SATA disk" ?  it isnt like disks are tiny and they have to scan a board
> > for some obscure IC.  disks are friggin huge.
>
> Come on. Have you never seen a system that came in a case, and you
> didn't know if it included a disk, or how many of these?

if it were a system that wasnt mine, sure.  but if i'm expected to actually 
develop on it, then it cant be a black box.  so no, i dont think it's 
unreasonable to have people who actually be using the hardware know what is in 
the system.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081223/d0075d2c/attachment.pgp 

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Wolfgang Denk]

Dear Mike Frysinger,

In message <200812111607.10866.vapier@gentoo.org> you wrote:
> 
> having init_sata() be automatic though would prevent hotplugging ... but maybe 
> people dont care about that ...

No - if you also allow to run "sata init" manually. the thing is that
it just should be done automatically if you use "normal" commands.

> there's no need to make u-boot unnecessarily safe for the people who dont 
> understand the system

But if we can fail without a crash, we should do.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Just because your doctor has a name for your condition  doesn't  mean
he knows what it is.

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Wolfgang Denk]

Dear Mike Frysinger,

In message <200812111556.23044.vapier@gentoo.org> you wrote:
>
> > Also, in the code you removed we do a runtime check on 8536 to see if
> > SATA is even available.  That check is still valid.
>
> why ?  if the hardware doesnt support it, then the user shouldnt be attempting 
> to use it.  if they do, that's their fault for doing something stupid.

And if they do, the code should fail gracefully, i. e. print some
friendly error message like that S-ATA is not available.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
2000 pounds of chinese soup                               = 1 Won Ton

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Mike Frysinger]

On Thursday 11 December 2008 18:53:00 Wolfgang Denk wrote:
> In message Mike Frysinger you wrote:
> > > Also, in the code you removed we do a runtime check on 8536 to see if
> > > SATA is even available.  That check is still valid.
> >
> > why ?  if the hardware doesnt support it, then the user shouldnt be
> > attempting to use it.  if they do, that's their fault for doing something
> > stupid.
>
> And if they do, the code should fail gracefully, i. e. print some
> friendly error message like that S-ATA is not available.

in this case, i think that's up to the controller.  i.e. the controller doesnt 
find anything and you get a message that nothing was found.

i dont have a board to test with to know what will happen exactly, nor does 
the current code make any statement that i can see.  Scott ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081211/d39a3496/attachment-0001.pgp 

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Scott Wood]

Mike Frysinger wrote:
> On Thursday 11 December 2008 18:53:00 Wolfgang Denk wrote:
>> And if they do, the code should fail gracefully, i. e. print some
>> friendly error message like that S-ATA is not available.
> 
> in this case, i think that's up to the controller.  i.e. the controller doesnt 
> find anything and you get a message that nothing was found.
> 
> i dont have a board to test with to know what will happen exactly, nor does 
> the current code make any statement that i can see.  Scott ?

IIUC, if it's "not available" the driver will still see the controller 
regs, but it will be trying to control pins that are actually routed to 
another device.  That said, it should be the driver calling into 
SoC-specific code to find out, rather than a global SATA-blocker (what 
if a PCIe SATA card were used?).

-Scott

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Mike Frysinger]

On Thursday 11 December 2008 19:15:06 Scott Wood wrote:
> Mike Frysinger wrote:
> > On Thursday 11 December 2008 18:53:00 Wolfgang Denk wrote:
> >> And if they do, the code should fail gracefully, i. e. print some
> >> friendly error message like that S-ATA is not available.
> >
> > in this case, i think that's up to the controller.  i.e. the controller
> > doesnt find anything and you get a message that nothing was found.
> >
> > i dont have a board to test with to know what will happen exactly, nor
> > does the current code make any statement that i can see.  Scott ?
>
> IIUC, if it's "not available" the driver will still see the controller
> regs, but it will be trying to control pins that are actually routed to
> another device.  That said, it should be the driver calling into
> SoC-specific code to find out, rather than a global SATA-blocker (what
> if a PCIe SATA card were used?).

i agree with that sentiment, but i'm not about to take on that restructure ;)
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081223/995af0c1/attachment.pgp 

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Wolfgang Denk]

Dear Kumar Gala,

In message <3B24DCFA-1309-4B92-A4AE-D9943D33269B@kernel.crashing.org> you wrote:
> 
> > and ...), so it isnt a big deal imo.  i asked if Wolfgang wanted it  
> > automated
> > and he preferred making the user do it themselves.
> 
> This seems backwards to me..

ACK here.

> 'sata init' isn't safe.  It seems like you should only be able to call  
> it once.  However I think we can keep issuing it and cause bad things  
> to happen.

No, I think on contrary it should explicitely be possible to call  it
repeatedly, for example when you (dis-) connect or replace devices on
the S-ATA interface(s).


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I distrust all systematisers, and avoid them. The will  to  a  system
shows a lack of honesty.
- Friedrich Wilhelm Nietzsche _G?tzen-D?mmerung [The Twilight of  the
Idols]_ ``Maxims and Missiles'' no. 26

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Wolfgang Denk]

Dear Mike Frysinger,

In message <200812111503.54148.vapier@gentoo.org> you wrote:
>
> it's already standard behavior with most mass storage things (like mmc and sf 
> and ...), so it isnt a big deal imo.  i asked if Wolfgang wanted it automated 
> and he preferred making the user do it themselves.

That's a misunderstanding, or my fingers typed without consulting the
brain. Sorry.

What I mean is: storage devices  should  auto-initialize  when  first
used  (like  for  example Ethernet does, too), but an explicit "init"
sub-command should be also available, so the user  can  re-initialize
the system if he likes.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Perl itself is  usually  pretty  good  about  telling  you  what  you
shouldn't do. :-)     - Larry Wall in <11091@jpl-devvax.JPL.NASA.GOV>

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Kumar Gala]

On Dec 11, 2008, at 4:32 AM, Mike Frysinger wrote:

> diff --git a/board/freescale/mpc8536ds/mpc8536ds.c b/board/freescale/ 
> mpc8536ds/mpc8536ds.c
> index 2b17612..ada8020 100644
> --- a/board/freescale/mpc8536ds/mpc8536ds.c
> +++ b/board/freescale/mpc8536ds/mpc8536ds.c
> @@ -582,17 +582,6 @@ get_board_ddr_clk(ulong dummy)
> }
> #endif
>
> -int is_sata_supported(void)
> -{
> -	volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
> -	uint sdrs2_io_sel =
> -		(gur->pordevsr & MPC85xx_PORDEVSR_SRDS2_IO_SEL) >> 27;
> -	if (sdrs2_io_sel & 0x04)
> -		return 0;
> -
> -	return 1;
> -}
> -

you removed this but didn't put it anywhere.. that's not going to work.

- k

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Mike Frysinger]

On Thursday 11 December 2008 11:18:36 Kumar Gala wrote:
> On Dec 11, 2008, at 4:32 AM, Mike Frysinger wrote:
> > diff --git a/board/freescale/mpc8536ds/mpc8536ds.c b/board/freescale/
> > mpc8536ds/mpc8536ds.c
> > index 2b17612..ada8020 100644
> > --- a/board/freescale/mpc8536ds/mpc8536ds.c
> > +++ b/board/freescale/mpc8536ds/mpc8536ds.c
> > @@ -582,17 +582,6 @@ get_board_ddr_clk(ulong dummy)
> > }
> > #endif
> >
> > -int is_sata_supported(void)
> > -{
> > -	volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
> > -	uint sdrs2_io_sel =
> > -		(gur->pordevsr & MPC85xx_PORDEVSR_SRDS2_IO_SEL) >> 27;
> > -	if (sdrs2_io_sel & 0x04)
> > -		return 0;
> > -
> > -	return 1;
> > -}
> > -
>
> you removed this but didn't put it anywhere.. that's not going to work.

of course it is going to work.  if nothing calls it, why should it exist in 
the first place ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081211/4f61d5be/attachment.pgp 

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Wolfgang Denk]

Dear Mike Frysinger,

In message <1228991549-11486-1-git-send-email-vapier@gentoo.org> you wrote:
> Rather than have the board code initialize SATA automatically during boot,
> make the user manually run "sata init".  This brings the SATA subsystem in
> line with common U-Boot policy.

I agree with Kumare - having to run "sata init" manually is annoying.
We should auto-run it upon the first "sata" command if it hasn't been
called before (but still keep the option  to  run  it  manually,  for
example   to   (re-)   initialize   the  system  after  changing  the
configuration while the system is running).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"A dirty mind is a joy forever."                       - Randy Kunkee

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Kumar Gala]

On Dec 11, 2008, at 5:18 PM, Wolfgang Denk wrote:

> Dear Mike Frysinger,
>
> In message <1228991549-11486-1-git-send-email-vapier@gentoo.org> you  
> wrote:
>> Rather than have the board code initialize SATA automatically  
>> during boot,
>> make the user manually run "sata init".  This brings the SATA  
>> subsystem in
>> line with common U-Boot policy.
>
> I agree with Kumare - having to run "sata init" manually is annoying.
> We should auto-run it upon the first "sata" command if it hasn't been
> called before (but still keep the option  to  run  it  manually,  for
> example   to   (re-)   initialize   the  system  after  changing  the
> configuration while the system is running).

But its not just sata commands.  I know I've used a SATA disk w/ext2  
load commands.

- k

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Wolfgang Denk]

Dear Kumar,

In message <29E3D6BF-AA38-40C3-AC21-87DCBBB82CAE@kernel.crashing.org> you wrote:
> 
> 
> > I agree with Kumare - having to run "sata init" manually is annoying.

Sorry for the bodus 'e' here.

> > We should auto-run it upon the first "sata" command if it hasn't been
> > called before (but still keep the option  to  run  it  manually,  for
> > example   to   (re-)   initialize   the  system  after  changing  the
> > configuration while the system is running).
> 
> But its not just sata commands.  I know I've used a SATA disk w/ext2  
> load commands.

But this will eventually call some S-ATA function  intenrally,w  hcih
can  check  that S-ATA has not been initialized yet, and run the init
function?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Every living thing wants to survive.
	-- Spock, "The Ultimate Computer", stardate 4731.3

[U-Boot] [PATCH] SATA: do not auto-initialize during boot

[Kumar Gala]

On Dec 11, 2008, at 6:01 PM, Wolfgang Denk wrote:

> Dear Kumar,
>
> In message <29E3D6BF-AA38-40C3- 
> AC21-87DCBBB82CAE at kernel.crashing.org> you wrote:
>>
>>
>>> I agree with Kumare - having to run "sata init" manually is  
>>> annoying.
>
> Sorry for the bodus 'e' here.
>
>>> We should auto-run it upon the first "sata" command if it hasn't  
>>> been
>>> called before (but still keep the option  to  run  it  manually,   
>>> for
>>> example   to   (re-)   initialize   the  system  after  changing   
>>> the
>>> configuration while the system is running).
>>
>> But its not just sata commands.  I know I've used a SATA disk w/ext2
>> load commands.
>
> But this will eventually call some S-ATA function  intenrally,w  hcih
> can  check  that S-ATA has not been initialized yet, and run the init
> function?

Ok.  Agreed if that is how Mike's patch ends up working.

- k

[U-Boot] SATA/IDE/etc... init

[Mike Frysinger]

btw, does nand_init() fall into the same category ?  should i convert that to 
a "nand init" step ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081211/45b755e7/attachment.pgp 

[U-Boot] SATA/IDE/etc... init

[Scott Wood]

On Thu, Dec 11, 2008 at 07:47:18AM -0500, Mike Frysinger wrote:
> btw, does nand_init() fall into the same category ? 

Hmm, in general I'd say yes, but that raises the issue with certain
drivers (such as fsl elbc) of how to communicate the ECC layout being
used to the kernel.  Currently (or at least, with a patch that was
posted) the kernel will leave u-boot's configuration alone.  Perhaps we
should add a device tree property.

> should i convert that to a "nand init" step ?

No, if initialization is deferred have it happen automatically when the
first real nand command is run.  We already defer the bad block scan in
this manner.

-Scott

[U-Boot] SATA/IDE/etc... init

[Wolfgang Denk]

Dear Mike Frysinger,

In message <200812110747.19343.vapier@gentoo.org> you wrote:
> 
> btw, does nand_init() fall into the same category ?  should i convert that to 
> a "nand init" step ?

Yes, it does.

I'm not sure here, though. Many people seem to find at least the NAND
flash size output useful.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Premature optimization is the root of all evil.         -- D.E. Knuth

[U-Boot] SATA/IDE/etc... init

[Mike Frysinger]

On Thursday 11 December 2008 18:22:06 Wolfgang Denk wrote:
> In message Mike Frysinger you wrote:
> > btw, does nand_init() fall into the same category ?  should i convert
> > that to a "nand init" step ?
>
> Yes, it does.
>
> I'm not sure here, though. Many people seem to find at least the NAND
> flash size output useful.

the biggest reason for me wanting this to die before was the bad block stuff, 
but as Scott said, that's been fixed (delayed) now

guess i'll leave it be since it doesnt really bug me anymore ;) ... if someone 
else cares, they can muck with it
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081211/42647ca7/attachment-0001.pgp 

[U-Boot] [PATCH v2] SATA: do not auto-initialize during boot

[Mike Frysinger]

Rather than have the board code initialize SATA automatically during boot,
make the user manually run "sata init".  This brings the SATA subsystem in
line with common U-Boot policy.

Along these lines, the "is_sata_supported()" hook is no longer needed, so
scrub it from the tree.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- auto run `sata init` if needed when doing other `sata ...` cmds

 board/freescale/mpc8536ds/mpc8536ds.c |   11 -----------
 common/cmd_sata.c                     |   11 ++++++++++-
 include/sata.h                        |    2 --
 lib_ppc/board.c                       |   20 --------------------
 4 files changed, 10 insertions(+), 34 deletions(-)

diff --git a/board/freescale/mpc8536ds/mpc8536ds.c b/board/freescale/mpc8536ds/mpc8536ds.c
index 2b17612..ada8020 100644
--- a/board/freescale/mpc8536ds/mpc8536ds.c
+++ b/board/freescale/mpc8536ds/mpc8536ds.c
@@ -582,17 +582,6 @@ get_board_ddr_clk(ulong dummy)
 }
 #endif
 
-int is_sata_supported(void)
-{
-	volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
-	uint sdrs2_io_sel =
-		(gur->pordevsr & MPC85xx_PORDEVSR_SRDS2_IO_SEL) >> 27;
-	if (sdrs2_io_sel & 0x04)
-		return 0;
-
-	return 1;
-}
-
 int board_eth_init(bd_t *bis)
 {
 #ifdef CONFIG_TSEC_ENET
diff --git a/common/cmd_sata.c b/common/cmd_sata.c
index dd6f1d9..83aee25 100644
--- a/common/cmd_sata.c
+++ b/common/cmd_sata.c
@@ -31,7 +31,7 @@
 int curr_device = -1;
 block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
 
-int sata_initialize(void)
+static int sata_initialize(void)
 {
 	int rc;
 	int i;
@@ -65,6 +65,14 @@ int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
 	int rc = 0;
 
+	if (argc == 2 && strcmp(argv[1], "init") == 0)
+		return sata_initialize();
+
+	/* If the user has not yet run `sata init`, do it now */
+	if (curr_device == -1)
+		if (sata_initialize())
+			return 1;
+
 	switch (argc) {
 	case 0:
 	case 1:
@@ -186,6 +194,7 @@ int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	sata, 5, 1, do_sata,
 	"sata	- SATA sub system\n",
+	"init - init SATA sub system\n"
 	"sata info - show available SATA devices\n"
 	"sata device [dev] - show or set current device\n"
 	"sata part [dev] - print partition table\n"
diff --git a/include/sata.h b/include/sata.h
index 57ee9ac..d07ca5a 100644
--- a/include/sata.h
+++ b/include/sata.h
@@ -6,6 +6,4 @@ int scan_sata(int dev);
 ulong sata_read(int dev, ulong blknr, ulong blkcnt, void *buffer);
 ulong sata_write(int dev, ulong blknr, ulong blkcnt, const void *buffer);
 
-int sata_initialize(void);
-
 #endif
diff --git a/lib_ppc/board.c b/lib_ppc/board.c
index 289a32a..3076824 100644
--- a/lib_ppc/board.c
+++ b/lib_ppc/board.c
@@ -38,9 +38,6 @@
 #if defined(CONFIG_CMD_IDE)
 #include <ide.h>
 #endif
-#if defined(CONFIG_CMD_SATA)
-#include <sata.h>
-#endif
 #if defined(CONFIG_CMD_SCSI)
 #include <scsi.h>
 #endif
@@ -635,16 +632,6 @@ void board_init_f (ulong bootflag)
 	/* NOTREACHED - relocate_code() does not return */
 }
 
-int __is_sata_supported(void)
-{
-	/* For some boards, when sata disabled by the switch, and the
-	 * driver still access the sata registers, the cpu will hangup.
-	 * please define platform specific is_sata_supported() if your
-	 * board have such issue.*/
-	return 1;
-}
-int is_sata_supported(void) __attribute__((weak, alias("__is_sata_supported")));
-
 /************************************************************************
  *
  * This is the next part if the initialization sequence: we are now
@@ -1144,13 +1131,6 @@ void board_init_r (gd_t *id, ulong dest_addr)
 #endif
 #endif
 
-#if defined(CONFIG_CMD_SATA)
-	if (is_sata_supported()) {
-		puts("SATA:  ");
-		sata_initialize();
-	}
-#endif
-
 #ifdef CONFIG_LAST_STAGE_INIT
 	WATCHDOG_RESET ();
 	/*
-- 
1.6.0.4

[U-Boot] [PATCH v2] SATA: do not auto-initialize during boot

[Kumar Gala]

On Dec 11, 2008, at 5:51 PM, Mike Frysinger wrote:

> Rather than have the board code initialize SATA automatically during  
> boot,
> make the user manually run "sata init".  This brings the SATA  
> subsystem in
> line with common U-Boot policy.
>
> Along these lines, the "is_sata_supported()" hook is no longer  
> needed, so
> scrub it from the tree.

Can we keep this and just move it into sata_initialize()?  It gives us  
the option on the board code (like on 8536 DS) to have proper runtime  
detection if the HW is available.  (ie as Scott said we might have a  
dip switch set on the board so SATA isn't enabled).

- k

[U-Boot] [PATCH v3] SATA: do not auto-initialize during boot

[Mike Frysinger]

Rather than have the board code initialize SATA automatically during boot,
make the user manually run "sata init".  This brings the SATA subsystem in
line with common U-Boot policy.

Rather than having a dedicated weak function "is_sata_supported", people
can override sata_initialize() to do their weird board stuff.  Then they
can call the actual __sata_initialize().

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v3
	- redo how the board weak is handled to be less bloated when
	  screwing over everyone else

v2
	- auto run `sata init` if needed when doing other `sata ...` cmds

 board/freescale/mpc8536ds/mpc8536ds.c |    6 +++---
 common/cmd_sata.c                     |   12 +++++++++++-
 include/sata.h                        |    1 +
 lib_ppc/board.c                       |   20 --------------------
 4 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/board/freescale/mpc8536ds/mpc8536ds.c b/board/freescale/mpc8536ds/mpc8536ds.c
index 2b17612..49474d7 100644
--- a/board/freescale/mpc8536ds/mpc8536ds.c
+++ b/board/freescale/mpc8536ds/mpc8536ds.c
@@ -582,15 +582,15 @@ get_board_ddr_clk(ulong dummy)
 }
 #endif
 
-int is_sata_supported(void)
+int sata_initialize(void)
 {
 	volatile ccsr_gur_t *gur = (void *)(CFG_MPC85xx_GUTS_ADDR);
 	uint sdrs2_io_sel =
 		(gur->pordevsr & MPC85xx_PORDEVSR_SRDS2_IO_SEL) >> 27;
 	if (sdrs2_io_sel & 0x04)
-		return 0;
+		return 1;
 
-	return 1;
+	return __sata_initialize();
 }
 
 #if defined(CONFIG_OF_BOARD_SETUP)
diff --git a/common/cmd_sata.c b/common/cmd_sata.c
index dd6f1d9..c6e0d37 100644
--- a/common/cmd_sata.c
+++ b/common/cmd_sata.c
@@ -31,7 +31,7 @@
 int curr_device = -1;
 block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
 
-int sata_initialize(void)
+int __sata_initialize(void)
 {
 	int rc;
 	int i;
@@ -55,6 +55,7 @@ int sata_initialize(void)
 	curr_device = 0;
 	return rc;
 }
+int sata_initialize(void) __attribute__((weak,alias("__sata_initialize")));
 
 block_dev_desc_t *sata_get_dev(int dev)
 {
@@ -65,6 +66,14 @@ int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
 	int rc = 0;
 
+	if (argc == 2 && strcmp(argv[1], "init") == 0)
+		return sata_initialize();
+
+	/* If the user has not yet run `sata init`, do it now */
+	if (curr_device == -1)
+		if (sata_initialize())
+			return 1;
+
 	switch (argc) {
 	case 0:
 	case 1:
@@ -186,6 +195,7 @@ int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	sata, 5, 1, do_sata,
 	"sata	- SATA sub system\n",
+	"init - init SATA sub system\n"
 	"sata info - show available SATA devices\n"
 	"sata device [dev] - show or set current device\n"
 	"sata part [dev] - print partition table\n"
diff --git a/include/sata.h b/include/sata.h
index 57ee9ac..37573cf 100644
--- a/include/sata.h
+++ b/include/sata.h
@@ -7,5 +7,6 @@ ulong sata_read(int dev, ulong blknr, ulong blkcnt, void *buffer);
 ulong sata_write(int dev, ulong blknr, ulong blkcnt, const void *buffer);
 
 int sata_initialize(void);
+int __sata_initialize(void);
 
 #endif
diff --git a/lib_ppc/board.c b/lib_ppc/board.c
index 289a32a..3076824 100644
--- a/lib_ppc/board.c
+++ b/lib_ppc/board.c
@@ -38,9 +38,6 @@
 #if defined(CONFIG_CMD_IDE)
 #include <ide.h>
 #endif
-#if defined(CONFIG_CMD_SATA)
-#include <sata.h>
-#endif
 #if defined(CONFIG_CMD_SCSI)
 #include <scsi.h>
 #endif
@@ -635,16 +632,6 @@ void board_init_f (ulong bootflag)
 	/* NOTREACHED - relocate_code() does not return */
 }
 
-int __is_sata_supported(void)
-{
-	/* For some boards, when sata disabled by the switch, and the
-	 * driver still access the sata registers, the cpu will hangup.
-	 * please define platform specific is_sata_supported() if your
-	 * board have such issue.*/
-	return 1;
-}
-int is_sata_supported(void) __attribute__((weak, alias("__is_sata_supported")));
-
 /************************************************************************
  *
  * This is the next part if the initialization sequence: we are now
@@ -1144,13 +1131,6 @@ void board_init_r (gd_t *id, ulong dest_addr)
 #endif
 #endif
 
-#if defined(CONFIG_CMD_SATA)
-	if (is_sata_supported()) {
-		puts("SATA:  ");
-		sata_initialize();
-	}
-#endif
-
 #ifdef CONFIG_LAST_STAGE_INIT
 	WATCHDOG_RESET ();
 	/*
-- 
1.6.0.4

[U-Boot] [PATCH v3] SATA: do not auto-initialize during boot

[Mike Frysinger]

On Tuesday 23 December 2008 00:10:39 Mike Frysinger wrote:
> Rather than have the board code initialize SATA automatically during boot,
> make the user manually run "sata init".  This brings the SATA subsystem in
> line with common U-Boot policy.
>
> Rather than having a dedicated weak function "is_sata_supported", people
> can override sata_initialize() to do their weird board stuff.  Then they
> can call the actual __sata_initialize().

i dont think anyone has a problem with this latest version (or at least, all 
the issues brought up before have been addressed), so can this be merged ?
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090123/6cb9f66d/attachment.pgp 

[U-Boot] [PATCH v3] SATA: do not auto-initialize during boot

[Wolfgang Denk]

Dear Mike Frysinger,

In message <1230009039-20117-1-git-send-email-vapier@gentoo.org> you wrote:
> Rather than have the board code initialize SATA automatically during boot,
> make the user manually run "sata init".  This brings the SATA subsystem in
> line with common U-Boot policy.
> 
> Rather than having a dedicated weak function "is_sata_supported", people
> can override sata_initialize() to do their weird board stuff.  Then they
> can call the actual __sata_initialize().
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> v3
> 	- redo how the board weak is handled to be less bloated when
> 	  screwing over everyone else
> 
> v2
> 	- auto run `sata init` if needed when doing other `sata ...` cmds
> 
>  board/freescale/mpc8536ds/mpc8536ds.c |    6 +++---
>  common/cmd_sata.c                     |   12 +++++++++++-
>  include/sata.h                        |    1 +
>  lib_ppc/board.c                       |   20 --------------------
>  4 files changed, 15 insertions(+), 24 deletions(-)

Sorry, this doesn;t apply any more:

error: patch failed: board/freescale/mpc8536ds/mpc8536ds.c:582
error: board/freescale/mpc8536ds/mpc8536ds.c: patch does not apply
Using index info to reconstruct a base tree...
error: patch failed: board/freescale/mpc8536ds/mpc8536ds.c:582
error: board/freescale/mpc8536ds/mpc8536ds.c: patch does not apply
Did you hand edit your patch?
It does not apply to blobs recorded in its index.
Cannot fall back to three-way merge.
Patch failed at 0001.


Please rebase and resubmit.  Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"A verbal contract isn't worth the paper it's printed on."
- Samuel Goldwyn

[U-Boot] [PATCH v4] SATA: do not auto-initialize during boot

[Mike Frysinger]

Rather than have the board code initialize SATA automatically during boot,
make the user manually run "sata init".  This brings the SATA subsystem in
line with common U-Boot policy.

Rather than having a dedicated weak function "is_sata_supported", people
can override sata_initialize() to do their weird board stuff.  Then they
can call the actual __sata_initialize().

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v4
	- rebase against latest master

v3
	- redo how the board weak is handled to be less bloated when
	  screwing over everyone else

v2
	- auto run `sata init` if needed when doing other `sata ...` 

 board/freescale/mpc8536ds/mpc8536ds.c |    6 +++---
 common/cmd_sata.c                     |   12 +++++++++++-
 include/sata.h                        |    1 +
 lib_ppc/board.c                       |   20 --------------------
 4 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/board/freescale/mpc8536ds/mpc8536ds.c b/board/freescale/mpc8536ds/mpc8536ds.c
index 1e2e2dc..1e2e4e6 100644
--- a/board/freescale/mpc8536ds/mpc8536ds.c
+++ b/board/freescale/mpc8536ds/mpc8536ds.c
@@ -582,15 +582,15 @@ get_board_ddr_clk(ulong dummy)
 }
 #endif
 
-int is_sata_supported(void)
+int sata_initialize(void)
 {
 	volatile ccsr_gur_t *gur = (void *)(CONFIG_SYS_MPC85xx_GUTS_ADDR);
 	uint sdrs2_io_sel =
 		(gur->pordevsr & MPC85xx_PORDEVSR_SRDS2_IO_SEL) >> 27;
 	if (sdrs2_io_sel & 0x04)
-		return 0;
+		return 1;
 
-	return 1;
+	return __sata_initialize();
 }
 
 int board_eth_init(bd_t *bis)
diff --git a/common/cmd_sata.c b/common/cmd_sata.c
index dd6f1d9..c6e0d37 100644
--- a/common/cmd_sata.c
+++ b/common/cmd_sata.c
@@ -31,7 +31,7 @@
 int curr_device = -1;
 block_dev_desc_t sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
 
-int sata_initialize(void)
+int __sata_initialize(void)
 {
 	int rc;
 	int i;
@@ -55,6 +55,7 @@ int sata_initialize(void)
 	curr_device = 0;
 	return rc;
 }
+int sata_initialize(void) __attribute__((weak,alias("__sata_initialize")));
 
 block_dev_desc_t *sata_get_dev(int dev)
 {
@@ -65,6 +66,14 @@ int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
 	int rc = 0;
 
+	if (argc == 2 && strcmp(argv[1], "init") == 0)
+		return sata_initialize();
+
+	/* If the user has not yet run `sata init`, do it now */
+	if (curr_device == -1)
+		if (sata_initialize())
+			return 1;
+
 	switch (argc) {
 	case 0:
 	case 1:
@@ -186,6 +195,7 @@ int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 U_BOOT_CMD(
 	sata, 5, 1, do_sata,
 	"sata	- SATA sub system\n",
+	"init - init SATA sub system\n"
 	"sata info - show available SATA devices\n"
 	"sata device [dev] - show or set current device\n"
 	"sata part [dev] - print partition table\n"
diff --git a/include/sata.h b/include/sata.h
index 57ee9ac..37573cf 100644
--- a/include/sata.h
+++ b/include/sata.h
@@ -7,5 +7,6 @@ ulong sata_read(int dev, ulong blknr, ulong blkcnt, void *buffer);
 ulong sata_write(int dev, ulong blknr, ulong blkcnt, const void *buffer);
 
 int sata_initialize(void);
+int __sata_initialize(void);
 
 #endif
diff --git a/lib_ppc/board.c b/lib_ppc/board.c
index 61c29b5..df1cf13 100644
--- a/lib_ppc/board.c
+++ b/lib_ppc/board.c
@@ -38,9 +38,6 @@
 #if defined(CONFIG_CMD_IDE)
 #include <ide.h>
 #endif
-#if defined(CONFIG_CMD_SATA)
-#include <sata.h>
-#endif
 #if defined(CONFIG_CMD_SCSI)
 #include <scsi.h>
 #endif
@@ -639,16 +636,6 @@ void board_init_f (ulong bootflag)
 	/* NOTREACHED - relocate_code() does not return */
 }
 
-int __is_sata_supported(void)
-{
-	/* For some boards, when sata disabled by the switch, and the
-	 * driver still access the sata registers, the cpu will hangup.
-	 * please define platform specific is_sata_supported() if your
-	 * board have such issue.*/
-	return 1;
-}
-int is_sata_supported(void) __attribute__((weak, alias("__is_sata_supported")));
-
 /************************************************************************
  *
  * This is the next part if the initialization sequence: we are now
@@ -1152,13 +1139,6 @@ void board_init_r (gd_t *id, ulong dest_addr)
 #endif
 #endif
 
-#if defined(CONFIG_CMD_SATA)
-	if (is_sata_supported()) {
-		puts("SATA:  ");
-		sata_initialize();
-	}
-#endif
-
 #ifdef CONFIG_LAST_STAGE_INIT
 	WATCHDOG_RESET ();
 	/*
-- 
1.6.1.1

[U-Boot] [PATCH v4] SATA: do not auto-initialize during boot

[Wolfgang Denk]

Dear Mike Frysinger,

In message <1233090741-3150-1-git-send-email-vapier@gentoo.org> you wrote:
> Rather than have the board code initialize SATA automatically during boot,
> make the user manually run "sata init".  This brings the SATA subsystem in
> line with common U-Boot policy.
> 
> Rather than having a dedicated weak function "is_sata_supported", people
> can override sata_initialize() to do their weird board stuff.  Then they
> can call the actual __sata_initialize().
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> v4
> 	- rebase against latest master
> 
> v3
> 	- redo how the board weak is handled to be less bloated when
> 	  screwing over everyone else
> 
> v2
> 	- auto run `sata init` if needed when doing other `sata ...` 
> 
>  board/freescale/mpc8536ds/mpc8536ds.c |    6 +++---
>  common/cmd_sata.c                     |   12 +++++++++++-
>  include/sata.h                        |    1 +
>  lib_ppc/board.c                       |   20 --------------------
>  4 files changed, 15 insertions(+), 24 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Only two things are infinite,  the universe and human stupidity,  and
I'm not sure about the former.                     -- Albert Einstein

[U-Boot] SATA/IDE/etc... init

[Stefan Roese]

On Friday 12 December 2008, Wolfgang Denk wrote:
> > btw, does nand_init() fall into the same category ?  should i convert
> > that to a "nand init" step ?
>
> Yes, it does.
>
> I'm not sure here, though. Many people seem to find at least the NAND
> flash size output useful.

Yes. And please don't forget that some boards, especially the ones with NAND 
booting support use NAND for environment storage. So nand_init() needs to be 
called here pretty early for env to work.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] SATA/IDE/etc... init

[Mike Frysinger]

On Friday 12 December 2008 00:53:36 Stefan Roese wrote:
> On Friday 12 December 2008, Wolfgang Denk wrote:
> > > btw, does nand_init() fall into the same category ?  should i convert
> > > that to a "nand init" step ?
> >
> > Yes, it does.
> >
> > I'm not sure here, though. Many people seem to find at least the NAND
> > flash size output useful.
>
> Yes. And please don't forget that some boards, especially the ones with
> NAND booting support use NAND for environment storage. So nand_init() needs
> to be called here pretty early for env to work.

nand env storage is irrelevant.  nand would be needed in this case so it would 
get initialized.  this is what happens for every other env backing (eeprom, 
sf, etc...).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20081222/44c54435/attachment.pgp