[PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard
Sergei Shtylyov
sshtylyov at mvista.com
Mon Dec 6 12:48:08 EST 2010
Hello.
Victor Rodriguez 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
... except you can't probably put a label next to }.
>>>>> 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_*'.
> I like it thanks I will submit the patches with that names. Sergei are
> you ok with this ?
I'm not sure the prefixes are necessary at all, but won't have objection to
those ones...
> Regards
> Victor Rodriguez
WBR, Sergei
More information about the linux-arm-kernel
mailing list