From mboxrd@z Thu Jan 1 00:00:00 1970 From: vm.rod25@gmail.com (Victor Rodriguez) Date: Mon, 6 Dec 2010 09:56:17 -0600 Subject: [PATCH v9 6/6] davinci: USB1.1 support for Omapl138-Hawkboard In-Reply-To: References: <1291231949-23835-1-git-send-email-vm.rod25@gmail.com> <1291231949-23835-7-git-send-email-vm.rod25@gmail.com> <4CFCCC56.3010700@mvista.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, Dec 6, 2010 at 9:53 AM, Ben Gardiner wrote: > On Mon, Dec 6, 2010 at 10:47 AM, Victor Rodriguez wrote: >> On Mon, Dec 6, 2010 at 5:43 AM, Sergei Shtylyov 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 >