Forgot the changelog:

On 29/10/14 17:56, Nikita Kiryanov wrote:

Change the order in which GPIOs are toggled in SATA init sequence to accomodate both SanDisk and Phison SSDs.

Signed-off-by: Nikita Kiryanov nikita@compulab.co.il Cc: Igor Grinberg grinberg@compulab.co.il Cc: Stefano Babic sbabic@denx.de

---

Changes in V2: - No changes.

board/compulab/cm_fx6/cm_fx6.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c index 82681b1..0206ae8 100644 --- a/board/compulab/cm_fx6/cm_fx6.c +++ b/board/compulab/cm_fx6/cm_fx6.c @@ -31,12 +31,12 @@ DECLARE_GLOBAL_DATA_PTR; #ifdef CONFIG_DWC_AHSATA static int cm_fx6_issd_gpios[] = { /* The order of the GPIOs in the array is important! */

• CM_FX6_SATA_LDO_EN, CM_FX6_SATA_PHY_SLP, CM_FX6_SATA_NRSTDLY, CM_FX6_SATA_PWREN, CM_FX6_SATA_NSTANDBY1, CM_FX6_SATA_NSTANDBY2,

• CM_FX6_SATA_LDO_EN, };

  static void cm_fx6_sata_power(int on)

Hi Nikita,

On 29/10/2014 16:56, Nikita Kiryanov wrote:

Introduce board specific function board_preboot_os() to allow for board specific config before we boot.

Signed-off-by: Nikita Kiryanov nikita@compulab.co.il Cc: Igor Grinberg grinberg@compulab.co.il Cc: Stefano Babic sbabic@denx.de Cc: Tom Rini trini@ti.com Cc: Jeroen Hofstee jeroen@myspectrum.nl Cc: Otavio Salvador otavio@ossystems.com.br

---

Changes in V2:

• Added board_preboot_os to bootm.h
• Split cm_fx6 stuff into a separate patch

common/bootm_os.c | 7 +++++++ include/bootm.h | 1 + 2 files changed, 8 insertions(+)

There is something that does not convince me. Really, the general statement should be to turn all devices off before booting the kernel, because this is what Linux expects. That said, this could be later solved with dm, because we will can iterate through all drivers and call a shutdown function.

Currently, "preboot" functions are turning off the hardware, so the name is already misleading. And I see that arch_preboot_os() was also convinced (because it is weak) to become a board_preboot, for example here: board/renesas/koelsch/koelsch.c (and in other renesas implementation, too).

I have some concerns adding a new weak function, that enables some further hooks inside board code. I find it not well scalable. In your case, you need such as sata_shutdown(), and it should be responsibility of the general sata code to call something specific for the board, if necessary.

In current code, bootm_disable_interrupts() calls also usb_stop() and eth_halt(), - nothing to do with the name of the function. IMHO it should be better to move the code to shutdown interface inside boot_selected_os(), or having a common function "disable_hardware()" that calls usb_stop(), eth_halt() and then, why not, a sata_stop().

There is also in arch/arm/imx-common/cpu.c a derived implementation for arch_preboot_os(). Really it has nothing to do with arch, as I can see: it shuts down only the IPU. But it is another hook, and IMHO it is better stopping sata here as adding a hidden callback.

Best regards, Stefano Babic

diff --git a/common/bootm_os.c b/common/bootm_os.c index 5be4467..95cd657 100644 --- a/common/bootm_os.c +++ b/common/bootm_os.c @@ -442,10 +442,17 @@ __weak void arch_preboot_os(void) /* please define platform specific arch_preboot_os() */ }

+/* Allow for board specific config before we boot */ +__weak void board_preboot_os(void) +{

• /* please define board specific board_preboot_os() */

+}

int boot_selected_os(int argc, char * const argv[], int state, bootm_headers_t *images, boot_os_fn *boot_fn) { arch_preboot_os();

• board_preboot_os(); boot_fn(state, argc, argv, images);

  /* Stand-alone may return when 'autostart' is 'no' */

diff --git a/include/bootm.h b/include/bootm.h index b3d1a62..7a57264 100644 --- a/include/bootm.h +++ b/include/bootm.h @@ -55,5 +55,6 @@ int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], int states, bootm_headers_t *images, int boot_progress);

void arch_preboot_os(void); +void board_preboot_os(void);

#endif