[U-Boot] [PATCH v3] MX28: Check if we are using a valid VBUS for power initialization

[Otavio Salvador]

On Sat, Aug 4, 2012 at 7:49 PM, Marek Vasut <marex at denx.de> wrote:
> Dear Otavio Salvador,
>
>> Signed-off-by: Otavio Salvador <otavio at ossystems.com.br>
>> ---
>> Changes in v2:
>> - add comments
>> - fix when we have vbus OR vdd5v
>> - improve patch short description
>>
>> Changes in v3:
>> - change short-description prefix
>>
>>  arch/arm/cpu/arm926ejs/mx28/spl_power_init.c |   26
>> +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7
>> deletions(-)
>>
>> diff --git a/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c
>> b/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c index 4b09b0c..fdf810c
>> 100644
>> --- a/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c
>> +++ b/arch/arm/cpu/arm926ejs/mx28/spl_power_init.c
>> @@ -564,6 +564,15 @@ void mx28_batt_boot(void)
>>               0x8 << POWER_5VCTRL_CHARGE_4P2_ILIMIT_OFFSET);
>>  }
>>
>> +static int mx28_valid_vbus(void)
>> +{
>> +     struct mx28_power_regs *power_regs =
>> +             (struct mx28_power_regs *)MXS_POWER_BASE;
>> +
>> +     /* iMX23 uses POWER_STS_VBUSVALID_STATUS at same offset */
>> +     return readl(&power_regs->hw_power_sts) & POWER_STS_VBUSVALID0_STATUS;
>> +}
>> +
>>  void mx28_handle_5v_conflict(void)
>>  {
>>       struct mx28_power_regs *power_regs =
>> @@ -577,14 +586,19 @@ void mx28_handle_5v_conflict(void)
>>               tmp = readl(&power_regs->hw_power_sts);
>>
>>               if (tmp & POWER_STS_VDDIO_BO) {
>> +                     /*
>> +                      * VDDIO has a brownout, then the VDD5V_GT_VDDIO becomes
>> +                      * unreliable
>> +                      */
>
> These should go into separate patch.

Ok; will split it.

>>                       mx28_powerdown();
>>                       break;
>>               }
>>
>> -             if (tmp & POWER_STS_VDD5V_GT_VDDIO) {
>> +             if (mx28_valid_vbus() || (tmp & POWER_STS_VDD5V_GT_VDDIO)) {
>
> And if VDD5V_GT_VDDIO isn't true, this will crash.

On bootlets mx28_valid_vbus() just returns 1 but the code of mx23 does
implement it and on the datasheet of mx28 it seems a valid check.

>>                       mx28_boot_valid_5v();
>>                       break;
>>               } else {
>> +                     /* Neither 5v sees 5v so we power down */
>>                       mx28_powerdown();
>>                       break;
>>               }
>> @@ -601,17 +615,15 @@ void mx28_5v_boot(void)
>>       struct mx28_power_regs *power_regs =
>>               (struct mx28_power_regs *)MXS_POWER_BASE;
>>
>> -     /*
>> -      * NOTE: In original IMX-Bootlets, this also checks for VBUSVALID,
>> -      * but their implementation always returns 1 so we omit it here.
>> -      */
>> -     if (readl(&power_regs->hw_power_sts) & POWER_STS_VDD5V_GT_VDDIO) {
>> +     if (mx28_valid_vbus() &&
>
> And again ... you unconditionally add something that will break other boards
> that aren't supplied from 5V. This part isn't present in mx28 bootlets if I'm
> right, yes?

Yes; this check is there too. But the comment about the difference
between mx23 and mx28 code is applied here too.

>> +             (readl(&power_regs->hw_power_sts) & POWER_STS_VDD5V_GT_VDDIO)) {
>>               mx28_boot_valid_5v();
>>               return;
>>       }
>>
>>       early_delay(1000);
>> -     if (readl(&power_regs->hw_power_sts) & POWER_STS_VDD5V_GT_VDDIO) {
>> +     if (mx28_valid_vbus() &&
>> +             (readl(&power_regs->hw_power_sts) & POWER_STS_VDD5V_GT_VDDIO)) {
>>               mx28_boot_valid_5v();
>>               return;
>>       }

Regards,

-- 
Otavio Salvador                             O.S. Systems
E-mail: otavio at ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br