[PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard

[Ben Gardiner]

On Mon, Dec 6, 2010 at 10:47 AM, Victor Rodriguez <vm.rod25 at gmail.com> wrote:
> On Mon, Dec 6, 2010 at 5:43 AM, Sergei Shtylyov <sshtylyov at mvista.com> wrote:
>> Hello.
>>
>> On 06-12-2010 14:12, Nori, Sekhar wrote:
>>
>>>>  arch/arm/mach-davinci/board-omapl138-hawk.c |   38
>>>> ++++++++++++++++++++------
>>>>  1 files changed, 29 insertions(+), 9 deletions(-)
>>
>>>> diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>> b/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>> index da51136..8fc78f2 100644
>>>> --- a/arch/arm/mach-davinci/board-omapl138-hawk.c
>>>> +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
>>
>> [...]
>>>>
>>>> @@ -165,13 +165,23 @@ static __init void omapl138_hawk_mmc_init(void)
>>>>       if (ret<  0) {
>>>>               pr_warning("%s: can not open GPIO %d\n",
>>>>                       __func__, DA850_HAWK_MMCSD_WP_PIN);
>>>> -             return;
>>>> +             goto exp_setup_wp_fail;
>>>>       }
>>>>
>>>>       ret = da8xx_register_mmcsd0(&da850_mmc_config);
>>>> -     if (ret)
>>>> +     if (ret) {
>>>>               pr_warning("%s: MMC/SD0 registration failed: %d\n",
>>>>                       __func__, ret);
>>>> +             goto exp_setup_mmcsd_fail;
>>>> +     }
>>>> +             return;
>>
>>> This return has extra indentation.
>>
>>>> +
>>>> +exp_setup_mmcsd_fail:
>>>> +     gpio_free(DA850_HAWK_MMCSD_WP_PIN);
>>>> +exp_setup_wp_fail:
>>>> +     gpio_free(DA850_HAWK_MMCSD_CD_PIN);
>>>> +exp_setup_cd_fail:
>>>> +             return;
>>
>>> This one too.
>>
>>   Moreover, it's not needed at all.
>
> Ok
>>> Other than that, it all looks good to me.
>>
>>   Except those 'exp_setup_'prefixes which I'm not sure where are coming
>> from...
>
>
> it comes from
>
> arch/arm/mach-davinci/board-da850-evm.c
>
> I took this as a template and I think that is better to keep this
> exp_setup_  as a template
>
> but if you have any other suggestion please tell me

I think those are based on the error handling in
da850_evm_ui_expander_setup() which means that the 'exp' in
'exp_setup_' stands-for IO-expander.

Based on the function names where the your exp_setup_* labels were
introduced I think that the label could be renamed 'mmc_setup_*' and
'usb11_setup_*'.


Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca