On 01/29/2014 03:07 PM, Dan Murphy wrote:
> 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 <dmur...@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 <dmur...@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
> 

OK. Thanks for the clarification.

cheers,
-roger
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to