On Mon, Dec 6, 2010 at 9:53 AM, Ben Gardiner <bengardi...@nanometrics.ca> wrote: > On Mon, Dec 6, 2010 at 10:47 AM, Victor Rodriguez <vm.ro...@gmail.com> wrote: >> On Mon, Dec 6, 2010 at 5:43 AM, Sergei Shtylyov <sshtyl...@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_*'. > I like it thanks I will submit the patches with that names. Sergei are you ok with this ?
Regards Victor Rodriguez > Best Regards, > Ben Gardiner > > --- > Nanometrics Inc. > http://www.nanometrics.ca > _______________________________________________ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source