[U-Boot] [PATCH 2/2] i.MX6: mx6q_sabrelite: add SATA bindings

[Eric Nelson <eric.nelson@boundarydevices.com>]

On 03/26/2012 09:59 AM, Eric Nelson wrote:
> On 03/26/2012 06:42 AM, Stefano Babic wrote:
>> On 26/03/2012 15:35, Eric Nelson wrote:
>>> On 03/26/2012 01:35 AM, Stefano Babic wrote:
>>>> On 26/03/2012 01:00, Eric Nelson wrote:
>>>>> V2 has been stripped of the board-independent changes and
>>>>>
>  >>>> <snip>
>  >>>>
>>>>> +#ifdef CONFIG_CMD_SATA
>>>>> +
>>>>> +int setup_sata(void)
>>>>> +{
>>>>> + int rval = enable_sata_clock();
>>>>
>>>> What about to return at this point if there is an error ?
>>>>
>>> I'm not sure I understand. Do you mean re-structure the code with
>>> two returns like this?
>>
>> No, much easier - I find the code is easy to understand if it looks like
>> if the function returns immediately in case of error.
>>
>> if (do_something())
>> return ERROR;
>>
>> < code when no error happens>
>>
>> Your enable_sata_clock() return only -1 in case of error. You could
>> easy write:
>>
>> if (enable_sata_clock())
>> return -1 (or better a value in errno.h)
>  >
> The choice of error code is better made inside enable_sata_clock(),
> although I'm not really sure what error code to choose.
>
> The error occurs if the PLL fails to lock and would indicate that
> something's horribly wrong.
>
> I'm guessing -EIO is probably the right choice.
>
> If you agree, I'll send V2 of "i.MX6: add enable_sata_clock()" and
> V3 of "i.MX6: mx6q_sabrelite: add SATA bindings".
>
Hi Stefano,

It looks like I was waiting for you and never did send V3 of this patch.

I'll forward one shortly.

Regards,


Eric