RE: [PATCH 3/9] ARM: EXYNOS: add initial setup-i2c0 for EXYNOS5
Olof Johansson wrote:
>
> Hi,
>
Hi :)
> On Tue, Jan 31, 2012 at 7:39 AM, Kukjin Kim wrote:
> > Signed-off-by: Kukjin Kim
>
> 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.
[...]
> > - * 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.
[...]
> > 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);
> > }
>
> 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 , Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/9] ARM: EXYNOS: add initial setup-i2c0 for EXYNOS5
Kyungmin Park wrote: > > On 2/1/12, Kukjin Kim wrote: [snip] > > + if (soc_is_exynos5250()) > > + ; > > + /* will be implemented with gpio function */ > Do you want to include both exynos4-gpio and exynos5-gpio? Now, yes. But I think, I need to sort out the regarding gpio later. Thanks. Best regards, Kgene. -- Kukjin Kim , Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/9] ARM: EXYNOS: add initial setup-i2c0 for EXYNOS5
Hi,
On Tue, Jan 31, 2012 at 7:39 AM, Kukjin Kim wrote:
> Signed-off-by: Kukjin Kim
A brief patch description, please.
> diff --git a/arch/arm/mach-exynos/setup-i2c0.c
> b/arch/arm/mach-exynos/setup-i2c0.c
> index d395bd1..3244f3e 100644
> --- a/arch/arm/mach-exynos/setup-i2c0.c
> +++ b/arch/arm/mach-exynos/setup-i2c0.c
> @@ -1,7 +1,7 @@
> /*
> - * 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.
> @@ -18,9 +18,14 @@ struct platform_device; /* don't need the contents */
> #include
> #include
> #include
> +#include
>
> 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);
> }
The above is pretty awkward. It's cleaner to return and avoid the else
side of the statement (move the comment accordingly, of course).
-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/9] ARM: EXYNOS: add initial setup-i2c0 for EXYNOS5
On 2/1/12, Kukjin Kim wrote:
> Signed-off-by: Kukjin Kim
> ---
> arch/arm/mach-exynos/Makefile |2 +-
> arch/arm/mach-exynos/setup-i2c0.c | 13 +
> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/Makefile b/arch/arm/mach-exynos/Makefile
> index 995e7cc..2117f02 100644
> --- a/arch/arm/mach-exynos/Makefile
> +++ b/arch/arm/mach-exynos/Makefile
> @@ -52,7 +52,7 @@ obj-$(CONFIG_EXYNOS4_DEV_DWMCI) += dev-dwmci.o
> obj-$(CONFIG_EXYNOS4_DEV_DMA)+= dma.o
> obj-$(CONFIG_EXYNOS4_DEV_USB_OHCI) += dev-ohci.o
>
> -obj-$(CONFIG_ARCH_EXYNOS4) += setup-i2c0.o
> +obj-$(CONFIG_ARCH_EXYNOS)+= setup-i2c0.o
> obj-$(CONFIG_EXYNOS4_SETUP_FIMC) += setup-fimc.o
> obj-$(CONFIG_EXYNOS4_SETUP_FIMD0)+= setup-fimd0.o
> obj-$(CONFIG_EXYNOS4_SETUP_I2C1) += setup-i2c1.o
> diff --git a/arch/arm/mach-exynos/setup-i2c0.c
> b/arch/arm/mach-exynos/setup-i2c0.c
> index d395bd1..3244f3e 100644
> --- a/arch/arm/mach-exynos/setup-i2c0.c
> +++ b/arch/arm/mach-exynos/setup-i2c0.c
> @@ -1,7 +1,7 @@
> /*
> - * linux/arch/arm/mach-exynos4/setup-i2c0.c
> + * linux/arch/arm/mach-exynos/setup-i2c0.c
> *
> - * Copyright (c) 2009-2010 Samsung Electronics Co., Ltd.
> + * Copyright (c) 2009-2012 Samsung Electronics Co., Ltd.
> * http://www.samsung.com/
> *
> * I2C0 GPIO configuration.
> @@ -18,9 +18,14 @@ struct platform_device; /* don't need the contents */
> #include
> #include
> #include
> +#include
>
> 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 */
Do you want to include both exynos4-gpio and exynos5-gpio?
> + else
> + s3c_gpio_cfgall_range(EXYNOS4_GPD1(0), 2,
> + S3C_GPIO_SFN(2), S3C_GPIO_PULL_UP);
> }
> --
> 1.7.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc"
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
