On Mon, Jan 3, 2011 at 8:18 PM, Sergei Shtylyov <[email protected]>wrote:
> Hello. > > On 03-01-2011 17:04, Subhasish Ghosh wrote: > > From: Subhasish <[email protected]> >> > > As far as I know, full name is needed. > [SG] -- I had given my full name, not sure why this happened, will check it out. > > Signed-off-by: Subhasish <[email protected]> >> > [...] > > diff --git a/arch/arm/mach-davinci/board-da850-evm.c >> b/arch/arm/mach-davinci/board-da850-evm.c >> index b01fb2a..f44d184 100644 >> --- a/arch/arm/mach-davinci/board-da850-evm.c >> +++ b/arch/arm/mach-davinci/board-da850-evm.c >> @@ -45,6 +45,7 @@ >> >> #define DA850_MMCSD_CD_PIN GPIO_TO_PIN(4, 0) >> #define DA850_MMCSD_WP_PIN GPIO_TO_PIN(4, 1) >> +#define DA850_PRU_CAN_TRX_PIN GPIO_TO_PIN(2, 0) >> > > Please align with the above and below macro values. > [SG] -- Will fix these. > > >> #define DA850_MII_MDIO_CLKEN_PIN GPIO_TO_PIN(2, 6) >> >> @@ -190,6 +191,41 @@ static struct platform_device *da850_evm_devices[] >> __initdata = { >> &da850_evm_norflash_device, >> }; >> >> +const short da850_pru_can_pins[] = { >> + DA850_PRU0_R31_0, DA850_PRU1_R30_15, DA850_PRU1_R31_18, >> + -1 >> +}; >> + >> +static int __init da850_evm_setup_pru_can(void) >> +{ >> + int ret; >> + >> + if (!machine_is_davinci_da850_evm()) >> + return 0; >> + >> + ret = davinci_cfg_reg_list(da850_pru_can_pins); >> + if (ret) >> + pr_warning("da850_evm_init: >> > > You're not in that functions. Please use __func__ variable to print the > function name instead. > [SG] -- Will clean these off. > > da850_pru_can_pins mux setup" >> > > You forgot space after thw word "setup". Your message will have > "setupfailed:" otherwise. > [SG] -- Will clean these off. > > + "failed:%d\n", ret); >> + >> + ret = davinci_cfg_reg(DA850_GPIO2_0); >> > > Just add this pin to da850_pru_can_pins[]. > [SG] -- Will do. > > + if (ret) >> + pr_warning("da850_evm_init: >> > > Same comment about printing the function name. > > GPIO(2,0) mux setup " >> + "failed\n"); >> > > It's strange that you don't print 'ret' here... > [SG] -- Will Do. > > + /* value = 0 to enable the CAN transceiver */ >> + ret = gpio_request_one(DA850_PRU_CAN_TRX_PIN, GPIOF_OUT_INIT_LOW, >> "pru_can_en"); >> + if (ret) >> + pr_warning("Cannot setup GPIO %d\n", >> DA850_PRU_CAN_TRX_PIN); >> + >> + ret = da8xx_register_pru_can(); >> + if (ret) >> + pr_warning("da850_evm_init: pru can registration failed:" >> > > Same comment about printing the function name. And please print acronyms > in all caps. > [SG] -- Will do. > > + "%d\n", ret); >> > > You also need to call gpio_free() on failure... > [SG] -- Will do. > > WBR, Sergei >
_______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
