[U-Boot] SATA/IDE/etc... init
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
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
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 (); /*
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
On Dec 11, 2008, at 6:01 PM, Wolfgang Denk wrote:
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?
Ok. Agreed if that is how Mike's patch ends up working.
- k
btw, does nand_init() fall into the same category ? should i convert that to a "nand init" step ? -mike
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
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
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
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 (); /*
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
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 (); /*
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
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
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 (); /*
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
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@denx.de =====================================================================
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
participants (5)
-
Kumar Gala -
Mike Frysinger -
Scott Wood -
Stefan Roese -
Wolfgang Denk