Olof Johansson wrote:

Hi,

Hi :)

On Tue, Jan 31, 2012 at 7:39 AM, Kukjin Kim [off-list ref] wrote:

quoted

Signed-off-by: Kukjin Kim <redacted>

A brief patch description, please.

OK. Will add, I thought we don't need same description with subject. But
what this patch is and why this patch is needed will be added next time.

[...]

quoted

- * linux/arch/arm/mach-exynos4/setup-i2c0.c
+ * linux/arch/arm/mach-exynos/setup-i2c0.c

The file name fills very little purpose in files like these. Consider
removing them at some point.

Yes, agree. Firstly, it will be removed when it is updated from now on.

[...]

quoted

?void s3c_i2c0_cfg_gpio(struct platform_device *dev)
?{
- ? ? ? s3c_gpio_cfgall_range(EXYNOS4_GPD1(0), 2,
- ? ? ? ? ? ? ? ? ? ? ? ? ? ? S3C_GPIO_SFN(2), S3C_GPIO_PULL_UP);
+ ? ? ? if (soc_is_exynos5250())
+ ? ? ? ? ? ? ? ;
+ ? ? ? ? ? ? ? /* will be implemented with gpio function */
+ ? ? ? else
+ ? ? ? ? ? ? ? s3c_gpio_cfgall_range(EXYNOS4_GPD1(0), 2,
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? S3C_GPIO_SFN(2),

S3C_GPIO_PULL_UP);

quoted

?}

The above is pretty awkward. It's cleaner to return and avoid the else
side of the statement (move the comment accordingly, of course).

OK.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim [off-list ref], Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.