diff mbox

[U-Boot] Convert CONFIG_SYS_I2C_OMAP24XX et al to Kconfig

Message ID 1501077817-5931-1-git-send-email-aford173@gmail.com
State Changes Requested
Delegated to: Tom Rini
Headers show

Commit Message

Adam Ford July 26, 2017, 2:03 p.m. UTC 
This converts the following to Kconfig:
   CONFIG_SYS_I2C_OMAP24XX
   CONFIG_SYS_I2C_OMAP34XX

Signed-off-by: Adam Ford <aford173@gmail.com>

Comments

Tom Rini July 27, 2017, 1:52 a.m. UTC | #1 
On Wed, Jul 26, 2017 at 09:03:37AM -0500, Adam Ford wrote:

> This converts the following to Kconfig:
>    CONFIG_SYS_I2C_OMAP24XX
>    CONFIG_SYS_I2C_OMAP34XX
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>

This needs some manual attention.  We should just drop
CONFIG_SYS_I2C_OMAP24XX as it's meaningless now.  Also:

> diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
> index 5476961..e15eaa6 100644
> --- a/include/configs/devkit8000.h
> +++ b/include/configs/devkit8000.h
> @@ -65,8 +65,6 @@
>  #undef CONFIG_OMAP3_SPI
>  
>  /* I2C */
> -#undef CONFIG_SYS_I2C_OMAP24XX
> -#define CONFIG_SYS_I2C_OMAP34XX

When you see cases like this, you should delete the comment (and extra
blank line) by hand as well, in the commit.  There's a few instances of
this in the patch.  Thanks!
Adam Ford July 27, 2017, 2:22 a.m. UTC | #2 
On Wed, Jul 26, 2017 at 8:52 PM, Tom Rini <trini@konsulko.com> wrote:
> On Wed, Jul 26, 2017 at 09:03:37AM -0500, Adam Ford wrote:
>
>> This converts the following to Kconfig:
>>    CONFIG_SYS_I2C_OMAP24XX
>>    CONFIG_SYS_I2C_OMAP34XX
>>
>> Signed-off-by: Adam Ford <aford173@gmail.com>
>
> This needs some manual attention.  We should just drop
> CONFIG_SYS_I2C_OMAP24XX as it's meaningless now.  Also:
>
I thought the same thing, but I looked at the driver and the driver
has some explicit differences that are unique to the
CONFIG_SYS_I2C_OMAP34XX.

As an example:
#if defined(CONFIG_OMAP34XX)
/*
* Have to enable interrupts for OMAP2/3, these IPs don't have
* an 'irqstatus_raw' register and we shall have to poll 'stat'
*/
writew(I2C_IE_XRDY_IE | I2C_IE_RRDY_IE | I2C_IE_ARDY_IE |
      I2C_IE_NACK_IE | I2C_IE_AL_IE, &i2c_base->ie);
#endif



The comment in the code even states there are some minor differences:
  "Status functions now read irqstatus_raw as per TRM guidelines
(except for OMAP243X and OMAP34XX)"

So I think we need both.
Looking at the ti_omap4_common.h, it defines  CONFIG_SYS_I2C_OMAP24XX,
but not OMAP34XX, so it appears to me like we might want a naming
convention change.

>> diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
>> index 5476961..e15eaa6 100644
>> --- a/include/configs/devkit8000.h
>> +++ b/include/configs/devkit8000.h
>> @@ -65,8 +65,6 @@
>>  #undef CONFIG_OMAP3_SPI
>>
>>  /* I2C */
>> -#undef CONFIG_SYS_I2C_OMAP24XX
>> -#define CONFIG_SYS_I2C_OMAP34XX
>
> When you see cases like this, you should delete the comment (and extra
> blank line) by hand as well, in the commit.  There's a few instances of
> this in the patch.  Thanks!

I can revisit this and fix it, but I'll wait to hear a reply to my
above comment.

adam
>
> --
> Tom
Tom Rini July 27, 2017, 11:55 a.m. UTC | #3 
On Wed, Jul 26, 2017 at 09:22:06PM -0500, Adam Ford wrote:
> On Wed, Jul 26, 2017 at 8:52 PM, Tom Rini <trini@konsulko.com> wrote:
> > On Wed, Jul 26, 2017 at 09:03:37AM -0500, Adam Ford wrote:
> >
> >> This converts the following to Kconfig:
> >>    CONFIG_SYS_I2C_OMAP24XX
> >>    CONFIG_SYS_I2C_OMAP34XX
> >>
> >> Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > This needs some manual attention.  We should just drop
> > CONFIG_SYS_I2C_OMAP24XX as it's meaningless now.  Also:
> >
> I thought the same thing, but I looked at the driver and the driver
> has some explicit differences that are unique to the
> CONFIG_SYS_I2C_OMAP34XX.
> 
> As an example:
> #if defined(CONFIG_OMAP34XX)
> /*
> * Have to enable interrupts for OMAP2/3, these IPs don't have
> * an 'irqstatus_raw' register and we shall have to poll 'stat'
> */
> writew(I2C_IE_XRDY_IE | I2C_IE_RRDY_IE | I2C_IE_ARDY_IE |
>       I2C_IE_NACK_IE | I2C_IE_AL_IE, &i2c_base->ie);
> #endif
> 
> 
> 
> The comment in the code even states there are some minor differences:
>   "Status functions now read irqstatus_raw as per TRM guidelines
> (except for OMAP243X and OMAP34XX)"
> 
> So I think we need both.
> Looking at the ti_omap4_common.h, it defines  CONFIG_SYS_I2C_OMAP24XX,
> but not OMAP34XX, so it appears to me like we might want a naming
> convention change.

But nothing toggles off of SYS_I2C_OMAP24XX vs SYS_I2C_OMAP34XX is the
key.  It might have back when we supported omap1/2 systems as well, but
it doesn't today.  Everything inside the driver keys off of
OMAP34XX/AM33XX/etc/etc now.
Adam Ford Aug. 6, 2017, 6:27 p.m. UTC | #4 
On Thu, Jul 27, 2017 at 6:55 AM, Tom Rini <trini@konsulko.com> wrote:
> On Wed, Jul 26, 2017 at 09:22:06PM -0500, Adam Ford wrote:
>> On Wed, Jul 26, 2017 at 8:52 PM, Tom Rini <trini@konsulko.com> wrote:
>> > On Wed, Jul 26, 2017 at 09:03:37AM -0500, Adam Ford wrote:
>> >
>> >> This converts the following to Kconfig:
>> >>    CONFIG_SYS_I2C_OMAP24XX
>> >>    CONFIG_SYS_I2C_OMAP34XX
>> >>
>> >> Signed-off-by: Adam Ford <aford173@gmail.com>
>> >
>> > This needs some manual attention.  We should just drop
>> > CONFIG_SYS_I2C_OMAP24XX as it's meaningless now.  Also:
>> >
>> I thought the same thing, but I looked at the driver and the driver
>> has some explicit differences that are unique to the
>> CONFIG_SYS_I2C_OMAP34XX.
>>
>> As an example:
>> #if defined(CONFIG_OMAP34XX)
>> /*
>> * Have to enable interrupts for OMAP2/3, these IPs don't have
>> * an 'irqstatus_raw' register and we shall have to poll 'stat'
>> */
>> writew(I2C_IE_XRDY_IE | I2C_IE_RRDY_IE | I2C_IE_ARDY_IE |
>>       I2C_IE_NACK_IE | I2C_IE_AL_IE, &i2c_base->ie);
>> #endif
>>
>>
>>
>> The comment in the code even states there are some minor differences:
>>   "Status functions now read irqstatus_raw as per TRM guidelines
>> (except for OMAP243X and OMAP34XX)"
>>
>> So I think we need both.
>> Looking at the ti_omap4_common.h, it defines  CONFIG_SYS_I2C_OMAP24XX,
>> but not OMAP34XX, so it appears to me like we might want a naming
>> convention change.
>
> But nothing toggles off of SYS_I2C_OMAP24XX vs SYS_I2C_OMAP34XX is the
> key.  It might have back when we supported omap1/2 systems as well, but
> it doesn't today.  Everything inside the driver keys off of
> OMAP34XX/AM33XX/etc/etc now.
>

Got it.  That makes sense.  Since the name of the source files is
omap24xx_i2c.c/.h , would you object to dumping the
CONFIG_SYS_I2C_OMAP34XX in favor of keeping the
CONFIG_SYS_I2C_OMAP24XX for consistency?

adam

> --
> Tom
Tom Rini Aug. 7, 2017, 3:25 p.m. UTC | #5 
On Sun, Aug 06, 2017 at 01:27:15PM -0500, Adam Ford wrote:
> On Thu, Jul 27, 2017 at 6:55 AM, Tom Rini <trini@konsulko.com> wrote:
> > On Wed, Jul 26, 2017 at 09:22:06PM -0500, Adam Ford wrote:
> >> On Wed, Jul 26, 2017 at 8:52 PM, Tom Rini <trini@konsulko.com> wrote:
> >> > On Wed, Jul 26, 2017 at 09:03:37AM -0500, Adam Ford wrote:
> >> >
> >> >> This converts the following to Kconfig:
> >> >>    CONFIG_SYS_I2C_OMAP24XX
> >> >>    CONFIG_SYS_I2C_OMAP34XX
> >> >>
> >> >> Signed-off-by: Adam Ford <aford173@gmail.com>
> >> >
> >> > This needs some manual attention.  We should just drop
> >> > CONFIG_SYS_I2C_OMAP24XX as it's meaningless now.  Also:
> >> >
> >> I thought the same thing, but I looked at the driver and the driver
> >> has some explicit differences that are unique to the
> >> CONFIG_SYS_I2C_OMAP34XX.
> >>
> >> As an example:
> >> #if defined(CONFIG_OMAP34XX)
> >> /*
> >> * Have to enable interrupts for OMAP2/3, these IPs don't have
> >> * an 'irqstatus_raw' register and we shall have to poll 'stat'
> >> */
> >> writew(I2C_IE_XRDY_IE | I2C_IE_RRDY_IE | I2C_IE_ARDY_IE |
> >>       I2C_IE_NACK_IE | I2C_IE_AL_IE, &i2c_base->ie);
> >> #endif
> >>
> >>
> >>
> >> The comment in the code even states there are some minor differences:
> >>   "Status functions now read irqstatus_raw as per TRM guidelines
> >> (except for OMAP243X and OMAP34XX)"
> >>
> >> So I think we need both.
> >> Looking at the ti_omap4_common.h, it defines  CONFIG_SYS_I2C_OMAP24XX,
> >> but not OMAP34XX, so it appears to me like we might want a naming
> >> convention change.
> >
> > But nothing toggles off of SYS_I2C_OMAP24XX vs SYS_I2C_OMAP34XX is the
> > key.  It might have back when we supported omap1/2 systems as well, but
> > it doesn't today.  Everything inside the driver keys off of
> > OMAP34XX/AM33XX/etc/etc now.
> >
> 
> Got it.  That makes sense.  Since the name of the source files is
> omap24xx_i2c.c/.h , would you object to dumping the
> CONFIG_SYS_I2C_OMAP34XX in favor of keeping the
> CONFIG_SYS_I2C_OMAP24XX for consistency?

That seems the most reasonable course, thanks!
diff mbox

Patch

diff --git a/configs/devkit8000_defconfig b/configs/devkit8000_defconfig
index f5e5317..6accc86 100644
--- a/configs/devkit8000_defconfig
+++ b/configs/devkit8000_defconfig
@@ -24,6 +24,7 @@  CONFIG_CMD_JFFS2=y
 CONFIG_CMD_MTDPARTS=y
 CONFIG_ISO_PARTITION=y
 CONFIG_EFI_PARTITION=y
+# CONFIG_SYS_I2C_OMAP24XX is not set
 CONFIG_MMC_OMAP_HS=y
 CONFIG_SYS_NS16550=y
 CONFIG_OF_LIBFDT=y
diff --git a/configs/omap3_zoom1_defconfig b/configs/omap3_zoom1_defconfig
index bf1ef98..2233cca 100644
--- a/configs/omap3_zoom1_defconfig
+++ b/configs/omap3_zoom1_defconfig
@@ -29,6 +29,7 @@  CONFIG_CMD_FS_GENERIC=y
 CONFIG_CMD_MTDPARTS=y
 CONFIG_ISO_PARTITION=y
 CONFIG_EFI_PARTITION=y
+# CONFIG_SYS_I2C_OMAP24XX is not set
 CONFIG_MMC_OMAP_HS=y
 CONFIG_SYS_NS16550=y
 CONFIG_FAT_WRITE=y
diff --git a/include/configs/am3517_crane.h b/include/configs/am3517_crane.h
index 77d9ba1..19f0ba4 100644
--- a/include/configs/am3517_crane.h
+++ b/include/configs/am3517_crane.h
@@ -107,7 +107,6 @@ 
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_OMAP24_I2C_SPEED	100000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
-#define CONFIG_SYS_I2C_OMAP34XX
 
 /*
  * Board NAND Info.
diff --git a/include/configs/am3517_evm.h b/include/configs/am3517_evm.h
index 26036c4..bfd51ee 100644
--- a/include/configs/am3517_evm.h
+++ b/include/configs/am3517_evm.h
@@ -94,7 +94,6 @@ 
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_OMAP24_I2C_SPEED	100000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
-#define CONFIG_SYS_I2C_OMAP34XX
 
 /* Ethernet */
 #define CONFIG_DRIVER_TI_EMAC
diff --git a/include/configs/bur_am335x_common.h b/include/configs/bur_am335x_common.h
index 8d0e0ea..c199d64 100644
--- a/include/configs/bur_am335x_common.h
+++ b/include/configs/bur_am335x_common.h
@@ -73,7 +73,6 @@ 
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_OMAP24_I2C_SPEED	100000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
-#define CONFIG_SYS_I2C_OMAP24XX
 
 /*
  * Our platforms make use of SPL to initalize the hardware (primarily
diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h
index 99d4800..0280f25 100644
--- a/include/configs/cm_t35.h
+++ b/include/configs/cm_t35.h
@@ -95,7 +95,6 @@ 
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_OMAP24_I2C_SPEED	100000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
-#define CONFIG_SYS_I2C_OMAP34XX
 #define CONFIG_SYS_I2C_EEPROM_ADDR	0x50
 #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	1
 #define CONFIG_SYS_I2C_EEPROM_BUS	0
diff --git a/include/configs/cm_t3517.h b/include/configs/cm_t3517.h
index 3fb6676..df8d11d 100644
--- a/include/configs/cm_t3517.h
+++ b/include/configs/cm_t3517.h
@@ -103,7 +103,6 @@ 
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_OMAP24_I2C_SPEED	400000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
-#define CONFIG_SYS_I2C_OMAP34XX
 #define CONFIG_SYS_I2C_EEPROM_ADDR	0x50
 #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	1
 #define CONFIG_SYS_I2C_EEPROM_BUS	0
diff --git a/include/configs/cm_t54.h b/include/configs/cm_t54.h
index 69706d2..638d9b9 100644
--- a/include/configs/cm_t54.h
+++ b/include/configs/cm_t54.h
@@ -17,7 +17,6 @@ 
 #include <configs/ti_omap5_common.h>
 
 /* EEPROM related defines */
-#define CONFIG_SYS_I2C_OMAP34XX
 #define CONFIG_SYS_I2C_EEPROM_ADDR	0x50
 #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	1
 #define CONFIG_SYS_I2C_EEPROM_BUS	0
diff --git a/include/configs/devkit8000.h b/include/configs/devkit8000.h
index 5476961..e15eaa6 100644
--- a/include/configs/devkit8000.h
+++ b/include/configs/devkit8000.h
@@ -65,8 +65,6 @@ 
 #undef CONFIG_OMAP3_SPI
 
 /* I2C */
-#undef CONFIG_SYS_I2C_OMAP24XX
-#define CONFIG_SYS_I2C_OMAP34XX
 
 /* TWL4030 */
 #define CONFIG_TWL4030_LED		1
diff --git a/include/configs/kc1.h b/include/configs/kc1.h
index f040d0b..38c5862 100644
--- a/include/configs/kc1.h
+++ b/include/configs/kc1.h
@@ -60,7 +60,6 @@ 
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_OMAP24_I2C_SPEED	400000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
-#define CONFIG_SYS_I2C_OMAP24XX
 #define CONFIG_I2C_MULTI_BUS
 
 /*
diff --git a/include/configs/mcx.h b/include/configs/mcx.h
index 73fdfbd..5279aad 100644
--- a/include/configs/mcx.h
+++ b/include/configs/mcx.h
@@ -88,7 +88,6 @@ 
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_OMAP24_I2C_SPEED	100000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
-#define CONFIG_SYS_I2C_OMAP34XX
 
 /* RTC */
 #define CONFIG_RTC_DS1337
diff --git a/include/configs/nokia_rx51.h b/include/configs/nokia_rx51.h
index 9826514..a06aec1 100644
--- a/include/configs/nokia_rx51.h
+++ b/include/configs/nokia_rx51.h
@@ -112,7 +112,6 @@ 
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_OMAP24_I2C_SPEED	100000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
-#define CONFIG_SYS_I2C_OMAP34XX
 
 /*
  * TWL4030
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h
index 15eb08b..8312337 100644
--- a/include/configs/omap3_evm.h
+++ b/include/configs/omap3_evm.h
@@ -150,7 +150,6 @@ 
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_OMAP24_I2C_SPEED	100000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
-#define CONFIG_SYS_I2C_OMAP34XX
 
 /*
  * PISMO support
diff --git a/include/configs/omap3_overo.h b/include/configs/omap3_overo.h
index 834822d..8372f43 100644
--- a/include/configs/omap3_overo.h
+++ b/include/configs/omap3_overo.h
@@ -34,7 +34,6 @@ 
 #define CONFIG_SYS_MALLOC_LEN	(CONFIG_ENV_SIZE + (128 << 15))
 
 /* I2C Support */
-#define CONFIG_SYS_I2C_OMAP34XX
 
 /* TWL4030 LED */
 #define CONFIG_TWL4030_LED
diff --git a/include/configs/omap3_pandora.h b/include/configs/omap3_pandora.h
index b7ab628..ec186a2 100644
--- a/include/configs/omap3_pandora.h
+++ b/include/configs/omap3_pandora.h
@@ -30,7 +30,6 @@ 
  */
 
 /* I2C Support */
-#define CONFIG_SYS_I2C_OMAP34XX
 
 /* TWL4030 LED */
 #define CONFIG_TWL4030_LED
diff --git a/include/configs/omap3_zoom1.h b/include/configs/omap3_zoom1.h
index 23603c6..db67a4d 100644
--- a/include/configs/omap3_zoom1.h
+++ b/include/configs/omap3_zoom1.h
@@ -64,9 +64,6 @@ 
 #define CONFIG_CMD_NAND_LOCK_UNLOCK /* Enable lock/unlock support */
 #endif
 
-#undef CONFIG_SYS_I2C_OMAP24XX
-#define CONFIG_SYS_I2C_OMAP34XX
-
 /*
  * TWL4030
  */
diff --git a/include/configs/siemens-am33x-common.h b/include/configs/siemens-am33x-common.h
index 22f070d..92febf3 100644
--- a/include/configs/siemens-am33x-common.h
+++ b/include/configs/siemens-am33x-common.h
@@ -104,7 +104,6 @@ 
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_OMAP24_I2C_SPEED	100000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
-#define CONFIG_SYS_I2C_OMAP24XX
 
 /* Defines for SPL */
 #define CONFIG_SPL_FRAMEWORK
diff --git a/include/configs/sniper.h b/include/configs/sniper.h
index 669ce85..34f6d8a 100644
--- a/include/configs/sniper.h
+++ b/include/configs/sniper.h
@@ -62,7 +62,6 @@ 
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_OMAP24_I2C_SPEED	400000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
-#define CONFIG_SYS_I2C_OMAP34XX
 #define CONFIG_I2C_MULTI_BUS
 
 /*
diff --git a/include/configs/tam3517-common.h b/include/configs/tam3517-common.h
index a9991fc..cc7a869 100644
--- a/include/configs/tam3517-common.h
+++ b/include/configs/tam3517-common.h
@@ -75,7 +75,6 @@ 
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_OMAP24_I2C_SPEED	400000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
-#define CONFIG_SYS_I2C_OMAP34XX
 #define CONFIG_SYS_I2C_EEPROM_ADDR	0x50		/* base address */
 #define CONFIG_SYS_I2C_EEPROM_ADDR_LEN	1		/* bytes of address */
 #define CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW	0x07
diff --git a/include/configs/tao3530.h b/include/configs/tao3530.h
index 38f5bd0..a00a023 100644
--- a/include/configs/tao3530.h
+++ b/include/configs/tao3530.h
@@ -71,7 +71,6 @@ 
 #define CONFIG_CMD_NAND		/* NAND support			*/
 
 #define CONFIG_SYS_I2C
-#define CONFIG_SYS_I2C_OMAP34XX
 #define CONFIG_SYS_OMAP24_I2C_SPEED	100000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
 #define CONFIG_I2C_MULTI_BUS
diff --git a/include/configs/ti_omap4_common.h b/include/configs/ti_omap4_common.h
index 1a6551e..acfac24 100644
--- a/include/configs/ti_omap4_common.h
+++ b/include/configs/ti_omap4_common.h
@@ -150,7 +150,6 @@ 
 #ifdef CONFIG_SPL_BUILD
 /* No need for i2c in SPL mode as we will use SRI2C for PMIC access on OMAP4 */
 #undef CONFIG_SYS_I2C
-#undef CONFIG_SYS_I2C_OMAP24XX
 #endif
 
 #endif /* __CONFIG_TI_OMAP4_COMMON_H */
diff --git a/include/configs/tricorder.h b/include/configs/tricorder.h
index a0fd583..941c8f0 100644
--- a/include/configs/tricorder.h
+++ b/include/configs/tricorder.h
@@ -62,7 +62,6 @@ 
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_OMAP24_I2C_SPEED	100000
 #define CONFIG_SYS_OMAP24_I2C_SLAVE	1
-#define CONFIG_SYS_I2C_OMAP34XX
  
 
 /* EEPROM */