[U-Boot] [uBoot 1/2] common: spl: Add spl sata boot support

[Dan Murphy]

Roger

On 01/29/2014 03:16 AM, Roger Quadros wrote:
> Hi Dan,
>
> On 01/28/2014 10:01 PM, Dan Murphy wrote:
>> Add spl_sata to read a fat partition from a bootable SATA
>> drive.
>>
>> Signed-off-by: Dan Murphy <dmurphy at ti.com>
>> ---
>>  common/Makefile       |    3 +++
>>  common/cmd_scsi.c     |    2 ++
>>  common/spl/Makefile   |    1 +
>>  common/spl/spl.c      |    5 +++++
>>  common/spl/spl_sata.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/spl.h         |    3 +++
>>  spl/Makefile          |    1 +
>>  7 files changed, 66 insertions(+)
>>  create mode 100644 common/spl/spl_sata.c
>>
>> diff --git a/common/Makefile b/common/Makefile
>> index 4d99ecd..b0f5b62 100644
>> --- a/common/Makefile
>> +++ b/common/Makefile
>> @@ -201,6 +201,9 @@ ifdef CONFIG_SPL_USB_HOST_SUPPORT
>>  obj-$(CONFIG_SPL_USB_SUPPORT) += usb.o usb_hub.o
>>  obj-$(CONFIG_USB_STORAGE) += usb_storage.o
>>  endif
>> +ifdef CONFIG_SPL_SATA_SUPPORT
>> +obj-$(CONFIG_CMD_SCSI) += cmd_scsi.o
>> +endif
>>  ifneq ($(CONFIG_SPL_NET_SUPPORT),y)
>>  obj-$(CONFIG_ENV_IS_NOWHERE) += env_nowhere.o
>>  obj-$(CONFIG_ENV_IS_IN_MMC) += env_mmc.o
>> diff --git a/common/cmd_scsi.c b/common/cmd_scsi.c
>> index 7b97dc9..b3f7687 100644
>> --- a/common/cmd_scsi.c
>> +++ b/common/cmd_scsi.c
>> @@ -168,7 +168,9 @@ removable:
>>  		scsi_curr_dev = -1;
>>  
>>  	printf("Found %d device(s).\n", scsi_max_devs);
>> +#ifndef CONFIG_SPL_BUILD
>>  	setenv_ulong("scsidevs", scsi_max_devs);
>> +#endif
>>  }
>>  
>>  int scsi_get_disk_count(void)
>> diff --git a/common/spl/Makefile b/common/spl/Makefile
>> index 65a1484f..64569c2 100644
>> --- a/common/spl/Makefile
>> +++ b/common/spl/Makefile
>> @@ -18,4 +18,5 @@ obj-$(CONFIG_SPL_NET_SUPPORT) += spl_net.o
>>  obj-$(CONFIG_SPL_MMC_SUPPORT) += spl_mmc.o
>>  obj-$(CONFIG_SPL_USB_SUPPORT) += spl_usb.o
>>  obj-$(CONFIG_SPL_FAT_SUPPORT) += spl_fat.o
>> +obj-$(CONFIG_SPL_SATA_SUPPORT) += spl_sata.o
>>  endif
>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>> index 0645cee..774fdad 100644
>> --- a/common/spl/spl.c
>> +++ b/common/spl/spl.c
>> @@ -210,6 +210,11 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>>  		spl_usb_load_image();
>>  		break;
>>  #endif
>> +#ifdef CONFIG_SPL_SATA_SUPPORT
>> +	case BOOT_DEVICE_SATA:
>> +		spl_sata_load_image();
>> +		break;
>> +#endif
>>  	default:
>>  		debug("SPL: Un-supported Boot Device\n");
>>  		hang();
>> diff --git a/common/spl/spl_sata.c b/common/spl/spl_sata.c
>> new file mode 100644
>> index 0000000..88d6b06
>> --- /dev/null
>> +++ b/common/spl/spl_sata.c
>> @@ -0,0 +1,51 @@
>> +/*
>> + * (C) Copyright 2013
>> + * Texas Instruments, <www.ti.com>
>> + *
>> + * Dan Murphy <dmurphy at ti.com>
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + *
>> + * Derived work from spl_usb.c
>> + */
>> +
>> +#include <common.h>
>> +#include <spl.h>
>> +#include <asm/u-boot.h>
>> +#include <sata.h>
>> +#include <fat.h>
>> +#include <version.h>
>> +#include <image.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +void spl_sata_load_image(void)
>> +{
>> +	int err;
>> +	block_dev_desc_t *stor_dev;
>> +
>> +	err = init_sata(CONFIG_SPL_SATA_BOOT_DEVICE);
>> +	if (err) {
>> +#ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
>> +		printf("spl: sata init failed: err - %d\n", err);
> Why the conditional print only if CONFIG_SPL_LIBCOMMON_SUPPORT.

As the file header indicates this is a copy/paste of the other SPL files.

>> +#endif
>> +		hang();
>> +	} else {
>> +		/* try to recognize storage devices immediately */
>> +		stor_dev = scsi_get_dev(0);
>> +	}
>> +
>> +	debug("boot mode - FAT\n");
> Do you really need this debug message? If yes could it be more descriptive like printing function name
> to point out where it comes from?

We really don't need this since only FAT is supported here.  I can remove it in v2

>> +
>> +#ifdef CONFIG_SPL_OS_BOOT
>> +	if (spl_start_uboot() || spl_load_image_fat_os(stor_dev,
>> +									CONFIG_SYS_SATA_FAT_BOOT_PARTITION))
>> +#endif
>> +	err = spl_load_image_fat(stor_dev,
> Do you need to tab space this to align with the above if statement?

Well that depends on if the ifdef is honored.  If the SPL_OS_BOOT is not defined then this statement is lined
up properly.  if it is defined then it is not.  Again this is the precedence for the other spl file (mmc and usb)

>
>> +				CONFIG_SYS_SATA_FAT_BOOT_PARTITION,
>> +				CONFIG_SPL_FAT_LOAD_PAYLOAD_NAME);
> Just cross checking. If CONFIG_SPL_OS_BOOT is defined then you need to call
> both spl_load_image_fat_os() as well as spl_load_image_fat ?

spl_load_image_fat is called within spl_load_image_fat_os.

spl_load_image_fat is only needed to be called if the other functions return a failure

>> +	if (err) {
>> +		puts("Error loading sata device\n");
>> +		hang();
>> +	}
>> +}
>> diff --git a/include/spl.h b/include/spl.h
>> index 5e24856..ee09fb6 100644
>> --- a/include/spl.h
>> +++ b/include/spl.h
>> @@ -64,6 +64,9 @@ void spl_net_load_image(const char *device);
>>  /* USB SPL functions */
>>  void spl_usb_load_image(void);
>>  
>> +/* SATA SPL functions */
>> +void spl_sata_load_image(void);
>> +
>>  /* SPL FAT image functions */
>>  int spl_load_image_fat(block_dev_desc_t *block_dev, int partition, const char *filename);
>>  int spl_load_image_fat_os(block_dev_desc_t *block_dev, int partition);
>> diff --git a/spl/Makefile b/spl/Makefile
>> index 4143e38..28fcfdd 100644
>> --- a/spl/Makefile
>> +++ b/spl/Makefile
>> @@ -85,6 +85,7 @@ LIBS-$(CONFIG_SPL_USBETH_SUPPORT) += drivers/usb/gadget/
>>  LIBS-$(CONFIG_SPL_WATCHDOG_SUPPORT) += drivers/watchdog/
>>  LIBS-$(CONFIG_SPL_USB_HOST_SUPPORT) += drivers/usb/host/
>>  LIBS-$(CONFIG_OMAP_USB_PHY) += drivers/usb/phy/
>> +LIBS-$(CONFIG_SPL_SATA_SUPPORT) += drivers/block/
>>  
>>  ifneq (,$(CONFIG_MX23)$(CONFIG_MX35)$(filter $(SOC), mx25 mx27 mx5 mx6 mx31 mx35))
>>  LIBS-y += arch/$(ARCH)/imx-common/
>>
> cheers,
> -roger


-- 
------------------
Dan Murphy