[PATCH v2 2/4] mx6cuboxi: customize board_boot_order to access eMMC

[Walter Lozano]

Hi Stefano,

I noticed that this series has state = Changes Requested, but not sure 
what are the changes need.

Could you please clarify? There is a silly hunk but Baruch suggested 
that this no requires a v3. However if you prefer a v3, I can prepare it

Please confirm.

On 16/3/20 15:56, Walter Lozano wrote:
> Hi Baruch,
>
> On 16/3/20 15:11, Baruch Siach wrote:
>> Hi Walter,
>>
>> On Mon, Mar 16, 2020 at 02:53:58PM -0300, Walter Lozano wrote:
>>> On 16/3/20 14:25, Baruch Siach wrote:
>>>> On Mon, Mar 16, 2020 at 02:05:57PM -0300, Walter Lozano wrote:
>>>>> On 16/3/20 13:28, Baruch Siach wrote:
>>>>>> On Thu, Mar 12, 2020 at 01:52:13PM -0300, Walter Lozano wrote:
>>>>>>> Thanks for sharing.
>>>>>>>
>>>>>>> On 12/3/20 02:02, Baruch Siach wrote:
>>>>>>>> Hi Walter,
>>>>>>>>
>>>>>>>> On Wed, Mar 11 2020, Walter Lozano wrote:
>>>>>>>>> In SPL legacy code only one MMC device is created, based on 
>>>>>>>>> BOOT_CFG
>>>>>>>>> register, which can be either SD or eMMC. In this context
>>>>>>>>> board_boot_order return always MMC1 when configure to boot from
>>>>>>>>> SD/eMMC. After switching to DM both SD and eMMC devices are 
>>>>>>>>> created
>>>>>>>>> based on the information available on DT, but as board_boot_order
>>>>>>>>> only returns MMC1 is not possible to boot from eMMC.
>>>>>>>>>
>>>>>>>>> This patch customizes board_boot_order taking into account 
>>>>>>>>> BOOT_CFG
>>>>>>>>> register to point to correct MMC1 / MMC2 device. Additionally, 
>>>>>>>>> handle
>>>>>>>>> IO mux for the desired boot device.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Walter Lozano <walter.lozano at collabora.com>
>>>>>>>>> ---
>>>>>>>>>      board/solidrun/mx6cuboxi/mx6cuboxi.c | 49 
>>>>>>>>> ++++++++++++++++++++++++++++
>>>>>>>>>      1 file changed, 49 insertions(+)
>>>>>>>>>
>>>>>>>>> diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c 
>>>>>>>>> b/board/solidrun/mx6cuboxi/mx6cuboxi.c
>>>>>>>>> index 6a96f9ecdb..9bf3645f72 100644
>>>>>>>>> --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
>>>>>>>>> +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
>>>>>>>>> @@ -435,6 +435,7 @@ int board_early_init_f(void)
>>>>>>>>>      #ifdef CONFIG_CMD_SATA
>>>>>>>>>          setup_sata();
>>>>>>>>>      #endif
>>>>>>>>> +
>>>>>>>> This hunk should not be part of this commit.
>>>>>>> Thanks for pointing to this silly hunk. I will prepare a V3.
>>>>>>>
>>>>>>>> Looks good to me, otherwise.
>>>>>>>>
>>>>>>>> I can't test at the moment. Have you tested boot from both SD 
>>>>>>>> card and eMMC?
>>>>>>> Most of the work was done booting from SD. In order to test 
>>>>>>> booting from
>>>>>>> eMMC, as I have some specific eFUSE configs, I tweaked 
>>>>>>> board_boot_order to
>>>>>>> force booting from eMMC.
>>>>>> But that does not cover SPL boot from eMMC, right?
>>>>> Basically I think this approach should cover the necessary steps. 
>>>>> To be more
>>>>> clear about my tweak
>>>>>
>>>>> 1- BootROM loads SPL from SD
>>>>>
>>>>> 2- SPL is tweaked to load U-Boot from eMMC, and in this way test 
>>>>> its support
>>>>> on SPL
>>>> This is not exactly the same as SPL boot from eMMC. For example, 
>>>> your scenario
>>>> would work even without 'u-boot,dm-pre-reloc' property in the eMMC 
>>>> device
>>>> node.
>>> I agree, it is not exactly the same and I really appreciate the time 
>>> you
>>> spent testing it. However I still don't understand your comments 
>>> regarding
>>> 'u-boot,dm-pre-reloc', as without this property there wouldn't be a 
>>> usdhc3
>>> node in the DTB for SPL. Could you please clarify?
>> You are right. Bad example.
>
> Thanks for clarifying.
>
>>
>>>>>> Anyway I tested your patches here on real hardware with unfused 
>>>>>> SOM and
>>>>>> SD/eMMC boot select jumpers.
>>>>> Thank you much for taking the time to test these patches in you 
>>>>> board. I
>>>>> really appreciate your help
>>>>>
>>>>>> Tested-by: Baruch Siach <baruch at tkos.co.il>
>>>>> Thanks. I'll add the tag to the v3.
>>>> I think this series ready as is. No need to post v3 just for the 
>>>> test tag.
>>>> Patchwork collects patch tags automatically. See under the 
>>>> 'A/F/R/T' column
>>>> here:
>>>>
>>>> http://patchwork.ozlabs.org/project/uboot/list/?series=163738
>>> I see, thanks for clarifying the issue related to "Tested-by" tag. 
>>> Sorry for
>>> asking but, is it not necessary to send a v3 to avoid the "silly 
>>> hunk" you
>>> pointed me?
>> I forgot about that. Maybe Stefano can make this trivial change when 
>> applying.
>> I would not respin the series just for that.
>
> Thanks again for clarifying, you have been very helpful. 

Regards,

Walter