diff mbox

[U-Boot,2/5] EXYNOS: Add Exynos4 I2C spacing

Message ID 1348469362-17314-3-git-send-email-p.wilczek@samsung.com
State Changes Requested
Delegated to: Minkyu Kang
Headers show

Commit Message

Piotr Wilczek Sept. 24, 2012, 6:49 a.m. UTC 
This patch add the spacing for i2c for Exynos4

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Minkyu Kang <mk7.kang@samsung.com>
---
 arch/arm/include/asm/arch-exynos/cpu.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Comments

Joon Young Shim Oct. 11, 2012, 1:10 a.m. UTC | #1 
Hi, Protr.

2012/9/24 Piotr Wilczek <p.wilczek@samsung.com>:
> This patch add the spacing for i2c for Exynos4
>
> Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> CC: Minkyu Kang <mk7.kang@samsung.com>
> ---
>  arch/arm/include/asm/arch-exynos/cpu.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-exynos/cpu.h b/arch/arm/include/asm/arch-exynos/cpu.h
> index 5056b37..b70273c 100644
> --- a/arch/arm/include/asm/arch-exynos/cpu.h
> +++ b/arch/arm/include/asm/arch-exynos/cpu.h
> @@ -28,6 +28,8 @@
>  #define EXYNOS4_ADDR_BASE              0x10000000
>
>  /* EXYNOS4 */
> +#define EXYNOS4_I2C_SPACING            0x10000
> +

Also EXYNOS5_I2C_SPACING is 0x10000, so how about define just
EXYNOS_I2C_SPACING for both EXYNOS4 and EXYNOS5?

>  #define EXYNOS4_GPIO_PART3_BASE                0x03860000
>  #define EXYNOS4_PRO_ID                 0x10000000
>  #define EXYNOS4_SYSREG_BASE            0x10010000
> --
> 1.7.5.4
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
Piotr Wilczek Oct. 11, 2012, 8:14 a.m. UTC | #2 
Hi Joonyoung,

> -----Original Message-----
> From: Joonyoung Shim [mailto:dofmind@gmail.com]
> Sent: Thursday, October 11, 2012 3:11 AM
> To: Piotr Wilczek
> Cc: u-boot@lists.denx.de; Kyungmin Park
> Subject: Re: [U-Boot] [PATCH 2/5] EXYNOS: Add Exynos4 I2C spacing
> 
> Hi, Protr.
> 
> 2012/9/24 Piotr Wilczek <p.wilczek@samsung.com>:
> > This patch add the spacing for i2c for Exynos4
> >
> > Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > CC: Minkyu Kang <mk7.kang@samsung.com>
> > ---
> >  arch/arm/include/asm/arch-exynos/cpu.h |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch-exynos/cpu.h
> > b/arch/arm/include/asm/arch-exynos/cpu.h
> > index 5056b37..b70273c 100644
> > --- a/arch/arm/include/asm/arch-exynos/cpu.h
> > +++ b/arch/arm/include/asm/arch-exynos/cpu.h
> > @@ -28,6 +28,8 @@
> >  #define EXYNOS4_ADDR_BASE              0x10000000
> >
> >  /* EXYNOS4 */
> > +#define EXYNOS4_I2C_SPACING            0x10000
> > +
> 
> Also EXYNOS5_I2C_SPACING is 0x10000, so how about define just
> EXYNOS_I2C_SPACING for both EXYNOS4 and EXYNOS5?
I agree it would simplify the code (s3c24x0_i2c driver). On the other hand,
for code readability, maybe it is better to keep them separate as we do so
with all defines for each Exynos.

> 
> >  #define EXYNOS4_GPIO_PART3_BASE                0x03860000
> >  #define EXYNOS4_PRO_ID                 0x10000000
> >  #define EXYNOS4_SYSREG_BASE            0x10010000
> > --
> > 1.7.5.4
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> 
> 
> --
> - Joonyoung Shim

Best regards,
Piotr Wilczek
Minkyu Kang Oct. 12, 2012, 2:35 a.m. UTC | #3 
Dear Piotr and Joonyoung,

On 11 October 2012 17:14, Piotr Wilczek <p.wilczek@samsung.com> wrote:
> Hi Joonyoung,
>
>> -----Original Message-----
>> From: Joonyoung Shim [mailto:dofmind@gmail.com]
>> Sent: Thursday, October 11, 2012 3:11 AM
>> To: Piotr Wilczek
>> Cc: u-boot@lists.denx.de; Kyungmin Park
>> Subject: Re: [U-Boot] [PATCH 2/5] EXYNOS: Add Exynos4 I2C spacing
>>
>> Hi, Protr.
>>
>> 2012/9/24 Piotr Wilczek <p.wilczek@samsung.com>:
>> > This patch add the spacing for i2c for Exynos4
>> >
>> > Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
>> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> > CC: Minkyu Kang <mk7.kang@samsung.com>
>> > ---
>> >  arch/arm/include/asm/arch-exynos/cpu.h |    2 ++
>> >  1 files changed, 2 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/arm/include/asm/arch-exynos/cpu.h
>> > b/arch/arm/include/asm/arch-exynos/cpu.h
>> > index 5056b37..b70273c 100644
>> > --- a/arch/arm/include/asm/arch-exynos/cpu.h
>> > +++ b/arch/arm/include/asm/arch-exynos/cpu.h
>> > @@ -28,6 +28,8 @@
>> >  #define EXYNOS4_ADDR_BASE              0x10000000
>> >
>> >  /* EXYNOS4 */
>> > +#define EXYNOS4_I2C_SPACING            0x10000
>> > +
>>
>> Also EXYNOS5_I2C_SPACING is 0x10000, so how about define just
>> EXYNOS_I2C_SPACING for both EXYNOS4 and EXYNOS5?
> I agree it would simplify the code (s3c24x0_i2c driver). On the other hand,
> for code readability, maybe it is better to keep them separate as we do so
> with all defines for each Exynos.
>

I agreed with Piotr.

Thanks
Minkyu Kang.
Joon Young Shim Oct. 12, 2012, 3:29 a.m. UTC | #4 
Hi,

2012/10/12 Minkyu Kang <promsoft@gmail.com>:
> Dear Piotr and Joonyoung,
>
> On 11 October 2012 17:14, Piotr Wilczek <p.wilczek@samsung.com> wrote:
>> Hi Joonyoung,
>>
>>> -----Original Message-----
>>> From: Joonyoung Shim [mailto:dofmind@gmail.com]
>>> Sent: Thursday, October 11, 2012 3:11 AM
>>> To: Piotr Wilczek
>>> Cc: u-boot@lists.denx.de; Kyungmin Park
>>> Subject: Re: [U-Boot] [PATCH 2/5] EXYNOS: Add Exynos4 I2C spacing
>>>
>>> Hi, Protr.
>>>
>>> 2012/9/24 Piotr Wilczek <p.wilczek@samsung.com>:
>>> > This patch add the spacing for i2c for Exynos4
>>> >
>>> > Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
>>> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> > CC: Minkyu Kang <mk7.kang@samsung.com>
>>> > ---
>>> >  arch/arm/include/asm/arch-exynos/cpu.h |    2 ++
>>> >  1 files changed, 2 insertions(+), 0 deletions(-)
>>> >
>>> > diff --git a/arch/arm/include/asm/arch-exynos/cpu.h
>>> > b/arch/arm/include/asm/arch-exynos/cpu.h
>>> > index 5056b37..b70273c 100644
>>> > --- a/arch/arm/include/asm/arch-exynos/cpu.h
>>> > +++ b/arch/arm/include/asm/arch-exynos/cpu.h
>>> > @@ -28,6 +28,8 @@
>>> >  #define EXYNOS4_ADDR_BASE              0x10000000
>>> >
>>> >  /* EXYNOS4 */
>>> > +#define EXYNOS4_I2C_SPACING            0x10000
>>> > +
>>>
>>> Also EXYNOS5_I2C_SPACING is 0x10000, so how about define just
>>> EXYNOS_I2C_SPACING for both EXYNOS4 and EXYNOS5?
>> I agree it would simplify the code (s3c24x0_i2c driver). On the other hand,
>> for code readability, maybe it is better to keep them separate as we do so
>> with all defines for each Exynos.
>>

OK, but already s3c24x0_i2c driver has too many #ifdef statements, so i
just concern it. We may need to use cpu_is_xxx to remove #ifdef.

Thanks.

>
> I agreed with Piotr.
>
> Thanks
> Minkyu Kang.
> --
> from. prom.
> www.promsoft.net
Minkyu Kang Oct. 12, 2012, 5:26 a.m. UTC | #5 
Dear Joonyoung,

On 12 October 2012 12:29, Joonyoung Shim <dofmind@gmail.com> wrote:
> Hi,
>
> 2012/10/12 Minkyu Kang <promsoft@gmail.com>:
>> Dear Piotr and Joonyoung,
>>
>> On 11 October 2012 17:14, Piotr Wilczek <p.wilczek@samsung.com> wrote:
>>> Hi Joonyoung,
>>>
>>>> -----Original Message-----
>>>> From: Joonyoung Shim [mailto:dofmind@gmail.com]
>>>> Sent: Thursday, October 11, 2012 3:11 AM
>>>> To: Piotr Wilczek
>>>> Cc: u-boot@lists.denx.de; Kyungmin Park
>>>> Subject: Re: [U-Boot] [PATCH 2/5] EXYNOS: Add Exynos4 I2C spacing
>>>>
>>>> Hi, Protr.
>>>>
>>>> 2012/9/24 Piotr Wilczek <p.wilczek@samsung.com>:
>>>> > This patch add the spacing for i2c for Exynos4
>>>> >
>>>> > Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
>>>> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>> > CC: Minkyu Kang <mk7.kang@samsung.com>
>>>> > ---
>>>> >  arch/arm/include/asm/arch-exynos/cpu.h |    2 ++
>>>> >  1 files changed, 2 insertions(+), 0 deletions(-)
>>>> >
>>>> > diff --git a/arch/arm/include/asm/arch-exynos/cpu.h
>>>> > b/arch/arm/include/asm/arch-exynos/cpu.h
>>>> > index 5056b37..b70273c 100644
>>>> > --- a/arch/arm/include/asm/arch-exynos/cpu.h
>>>> > +++ b/arch/arm/include/asm/arch-exynos/cpu.h
>>>> > @@ -28,6 +28,8 @@
>>>> >  #define EXYNOS4_ADDR_BASE              0x10000000
>>>> >
>>>> >  /* EXYNOS4 */
>>>> > +#define EXYNOS4_I2C_SPACING            0x10000
>>>> > +
>>>>
>>>> Also EXYNOS5_I2C_SPACING is 0x10000, so how about define just
>>>> EXYNOS_I2C_SPACING for both EXYNOS4 and EXYNOS5?
>>> I agree it would simplify the code (s3c24x0_i2c driver). On the other hand,
>>> for code readability, maybe it is better to keep them separate as we do so
>>> with all defines for each Exynos.
>>>
>
> OK, but already s3c24x0_i2c driver has too many #ifdef statements, so i
> just concern it. We may need to use cpu_is_xxx to remove #ifdef.
>

Right.
We should do cleanup.
Actually, I don't want to use such a define and ifdefs.. like other devices.
I think, we can solve this problem by separate i2c driver from s3c24x0.
How you think?
Piotr Wilczek Oct. 12, 2012, 6:47 a.m. UTC | #6 
Dear Minkyu and Joonyoung,

> -----Original Message-----
> From: Minkyu Kang [mailto:promsoft@gmail.com]
> Sent: Friday, October 12, 2012 7:26 AM
> To: Joonyoung Shim
> Cc: Piotr Wilczek; u-boot@lists.denx.de; Kyungmin Park
> Subject: Re: [U-Boot] [PATCH 2/5] EXYNOS: Add Exynos4 I2C spacing
> 
> Dear Joonyoung,
> 
> On 12 October 2012 12:29, Joonyoung Shim <dofmind@gmail.com> wrote:
> > Hi,
> >
> > 2012/10/12 Minkyu Kang <promsoft@gmail.com>:
> >> Dear Piotr and Joonyoung,
> >>
> >> On 11 October 2012 17:14, Piotr Wilczek <p.wilczek@samsung.com>
> wrote:
> >>> Hi Joonyoung,
> >>>
> >>>> -----Original Message-----
> >>>> From: Joonyoung Shim [mailto:dofmind@gmail.com]
> >>>> Sent: Thursday, October 11, 2012 3:11 AM
> >>>> To: Piotr Wilczek
> >>>> Cc: u-boot@lists.denx.de; Kyungmin Park
> >>>> Subject: Re: [U-Boot] [PATCH 2/5] EXYNOS: Add Exynos4 I2C spacing
> >>>>
> >>>> Hi, Protr.
> >>>>
> >>>> 2012/9/24 Piotr Wilczek <p.wilczek@samsung.com>:
> >>>> > This patch add the spacing for i2c for Exynos4
> >>>> >
> >>>> > Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
> >>>> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >>>> > CC: Minkyu Kang <mk7.kang@samsung.com>
> >>>> > ---
> >>>> >  arch/arm/include/asm/arch-exynos/cpu.h |    2 ++
> >>>> >  1 files changed, 2 insertions(+), 0 deletions(-)
> >>>> >
> >>>> > diff --git a/arch/arm/include/asm/arch-exynos/cpu.h
> >>>> > b/arch/arm/include/asm/arch-exynos/cpu.h
> >>>> > index 5056b37..b70273c 100644
> >>>> > --- a/arch/arm/include/asm/arch-exynos/cpu.h
> >>>> > +++ b/arch/arm/include/asm/arch-exynos/cpu.h
> >>>> > @@ -28,6 +28,8 @@
> >>>> >  #define EXYNOS4_ADDR_BASE              0x10000000
> >>>> >
> >>>> >  /* EXYNOS4 */
> >>>> > +#define EXYNOS4_I2C_SPACING            0x10000
> >>>> > +
> >>>>
> >>>> Also EXYNOS5_I2C_SPACING is 0x10000, so how about define just
> >>>> EXYNOS_I2C_SPACING for both EXYNOS4 and EXYNOS5?
> >>> I agree it would simplify the code (s3c24x0_i2c driver). On the
> >>> other hand, for code readability, maybe it is better to keep them
> >>> separate as we do so with all defines for each Exynos.
> >>>
> >
> > OK, but already s3c24x0_i2c driver has too many #ifdef statements, so
> > i just concern it. We may need to use cpu_is_xxx to remove #ifdef.
> >
> 
> Right.
> We should do cleanup.
> Actually, I don't want to use such a define and ifdefs.. like other
> devices.
> I think, we can solve this problem by separate i2c driver from s3c24x0.
> How you think?

I agree that the driver is too complicated and we should separate i2c from
s3c24x0. I just thought it could be done in a separate patchset. However, if
you think it should be done now, I will do so.

> 
> --
> from. prom.
> www.promsoft.net

Best regards,
Piotr Wilczek
diff mbox

Patch

diff --git a/arch/arm/include/asm/arch-exynos/cpu.h b/arch/arm/include/asm/arch-exynos/cpu.h
index 5056b37..b70273c 100644
--- a/arch/arm/include/asm/arch-exynos/cpu.h
+++ b/arch/arm/include/asm/arch-exynos/cpu.h
@@ -28,6 +28,8 @@ 
 #define EXYNOS4_ADDR_BASE		0x10000000
 
 /* EXYNOS4 */
+#define EXYNOS4_I2C_SPACING		0x10000
+
 #define EXYNOS4_GPIO_PART3_BASE		0x03860000
 #define EXYNOS4_PRO_ID			0x10000000
 #define EXYNOS4_SYSREG_BASE		0x10010000