[v2,6/8] ARM: Use Kconfig for board EEPROM's I2C bus and chip address

Message ID 1485789316-22216-7-git-send-email-rogerq@ti.com
State New
Headers show
Series
  • am57xx-idk LCD and am571x-idk 6 port ethernet pinmux
Related show

Commit Message

Roger Quadros Jan. 30, 2017, 3:15 p.m.
In stead of defining the board EEPROM address in the board headers
let's define them in the board config files and make them
configurable by Kconfig.

Signed-off-by: Roger Quadros <rogerq@ti.com>

---
 arch/arm/Kconfig                  | 10 ++++++++++
 configs/am57xx_evm_defconfig      |  2 ++
 configs/am57xx_evm_nodt_defconfig |  2 ++
 configs/am57xx_hs_evm_defconfig   |  2 ++
 configs/dra7xx_evm_defconfig      |  2 ++
 configs/dra7xx_hs_evm_defconfig   |  2 ++
 include/configs/am57xx_evm.h      |  4 ----
 include/configs/dra7xx_evm.h      |  4 ----
 8 files changed, 20 insertions(+), 8 deletions(-)

-- 
2.7.4

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Comments

Lokesh Vutla Jan. 31, 2017, 4:17 a.m. | #1
On Monday 30 January 2017 08:45 PM, Roger Quadros wrote:
> In stead of defining the board EEPROM address in the board headers

> let's define them in the board config files and make them

> configurable by Kconfig.

> 

> Signed-off-by: Roger Quadros <rogerq@ti.com>

> ---

>  arch/arm/Kconfig                  | 10 ++++++++++

>  configs/am57xx_evm_defconfig      |  2 ++

>  configs/am57xx_evm_nodt_defconfig |  2 ++

>  configs/am57xx_hs_evm_defconfig   |  2 ++

>  configs/dra7xx_evm_defconfig      |  2 ++

>  configs/dra7xx_hs_evm_defconfig   |  2 ++

>  include/configs/am57xx_evm.h      |  4 ----

>  include/configs/dra7xx_evm.h      |  4 ----

>  8 files changed, 20 insertions(+), 8 deletions(-)

> 

> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig

> index c04adfb..19886ad 100644

> --- a/arch/arm/Kconfig

> +++ b/arch/arm/Kconfig

> @@ -1009,6 +1009,16 @@ config TARGET_THUNDERX_88XX

>  

>  endchoice

>  

> +config EEPROM_BUS_ADDRESS

> +        int "Board EEPROM's I2C bus address"

> +        range 0 8

> +        default 0

> +

> +config EEPROM_CHIP_ADDRESS

> +        hex "Board EEPROM's I2C chip address"

> +        range 0 0xff

> +        default 0x50

> +


IMO, these should be placed in board/ti/common/Kconfig. Also need not
specify the value explicitly in defconfig if default value is used.

Thanks and regards,
Lokesh

>  source "arch/arm/mach-at91/Kconfig"

>  

>  source "arch/arm/mach-bcm283x/Kconfig"

> diff --git a/configs/am57xx_evm_defconfig b/configs/am57xx_evm_defconfig

> index 656d991..3b3e74a 100644

> --- a/configs/am57xx_evm_defconfig

> +++ b/configs/am57xx_evm_defconfig

> @@ -2,6 +2,8 @@ CONFIG_ARM=y

>  CONFIG_OMAP54XX=y

>  # CONFIG_SPL_NAND_SUPPORT is not set

>  CONFIG_TARGET_AM57XX_EVM=y

> +CONFIG_EEPROM_BUS_ADDRESS=0

> +CONFIG_EEPROM_CHIP_ADDRESS=0x50

>  CONFIG_SPL_SPI_FLASH_SUPPORT=y

>  CONFIG_SPL_SPI_SUPPORT=y

>  CONFIG_ARMV7_LPAE=y

> diff --git a/configs/am57xx_evm_nodt_defconfig b/configs/am57xx_evm_nodt_defconfig

> index b3b95f9..8d722e7 100644

> --- a/configs/am57xx_evm_nodt_defconfig

> +++ b/configs/am57xx_evm_nodt_defconfig

> @@ -2,6 +2,8 @@ CONFIG_ARM=y

>  CONFIG_OMAP54XX=y

>  # CONFIG_SPL_NAND_SUPPORT is not set

>  CONFIG_TARGET_AM57XX_EVM=y

> +CONFIG_EEPROM_BUS_ADDRESS=0

> +CONFIG_EEPROM_CHIP_ADDRESS=0x50

>  CONFIG_SPL_SPI_FLASH_SUPPORT=y

>  CONFIG_SPL_SPI_SUPPORT=y

>  CONFIG_SYS_CONSOLE_INFO_QUIET=y

> diff --git a/configs/am57xx_hs_evm_defconfig b/configs/am57xx_hs_evm_defconfig

> index d920d68..36c8004 100644

> --- a/configs/am57xx_hs_evm_defconfig

> +++ b/configs/am57xx_hs_evm_defconfig

> @@ -3,6 +3,8 @@ CONFIG_OMAP54XX=y

>  CONFIG_TI_SECURE_DEVICE=y

>  # CONFIG_SPL_NAND_SUPPORT is not set

>  CONFIG_TARGET_AM57XX_EVM=y

> +CONFIG_EEPROM_BUS_ADDRESS=0

> +CONFIG_EEPROM_CHIP_ADDRESS=0x50

>  CONFIG_TI_SECURE_EMIF_REGION_START=0xbe000000

>  CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE=0x02000000

>  CONFIG_TI_SECURE_EMIF_PROTECTED_REGION_SIZE=0x01c00000

> diff --git a/configs/dra7xx_evm_defconfig b/configs/dra7xx_evm_defconfig

> index 1836021..3108c58 100644

> --- a/configs/dra7xx_evm_defconfig

> +++ b/configs/dra7xx_evm_defconfig

> @@ -2,6 +2,8 @@ CONFIG_ARM=y

>  CONFIG_OMAP54XX=y

>  # CONFIG_SPL_NAND_SUPPORT is not set

>  CONFIG_TARGET_DRA7XX_EVM=y

> +CONFIG_EEPROM_BUS_ADDRESS=0

> +CONFIG_EEPROM_CHIP_ADDRESS=0x50

>  CONFIG_SPL_SPI_FLASH_SUPPORT=y

>  CONFIG_SPL_SPI_SUPPORT=y

>  CONFIG_ARMV7_LPAE=y

> diff --git a/configs/dra7xx_hs_evm_defconfig b/configs/dra7xx_hs_evm_defconfig

> index 1d107e4..7f5ce49 100644

> --- a/configs/dra7xx_hs_evm_defconfig

> +++ b/configs/dra7xx_hs_evm_defconfig

> @@ -4,6 +4,8 @@ CONFIG_SYS_MALLOC_F_LEN=0x2000

>  CONFIG_TI_SECURE_DEVICE=y

>  # CONFIG_SPL_NAND_SUPPORT is not set

>  CONFIG_TARGET_DRA7XX_EVM=y

> +CONFIG_EEPROM_BUS_ADDRESS=0

> +CONFIG_EEPROM_CHIP_ADDRESS=0x50

>  CONFIG_TI_SECURE_EMIF_REGION_START=0xbe000000

>  CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE=0x02000000

>  CONFIG_TI_SECURE_EMIF_PROTECTED_REGION_SIZE=0x01c00000

> diff --git a/include/configs/am57xx_evm.h b/include/configs/am57xx_evm.h

> index 840502c..d9e1119 100644

> --- a/include/configs/am57xx_evm.h

> +++ b/include/configs/am57xx_evm.h

> @@ -105,10 +105,6 @@

>  #define CONFIG_SYS_SCSI_MAX_DEVICE	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \

>  						CONFIG_SYS_SCSI_MAX_LUN)

>  

> -/* EEPROM */

> -#define CONFIG_EEPROM_CHIP_ADDRESS 0x50

> -#define CONFIG_EEPROM_BUS_ADDRESS 0

> -

>  /*

>   * Default to using SPI for environment, etc.

>   * 0x000000 - 0x040000 : QSPI.SPL (256KiB)

> diff --git a/include/configs/dra7xx_evm.h b/include/configs/dra7xx_evm.h

> index da458a4..46beb8b 100644

> --- a/include/configs/dra7xx_evm.h

> +++ b/include/configs/dra7xx_evm.h

> @@ -264,8 +264,4 @@

>  #endif

>  #endif  /* NOR support */

>  

> -/* EEPROM */

> -#define CONFIG_EEPROM_CHIP_ADDRESS 0x50

> -#define CONFIG_EEPROM_BUS_ADDRESS 0

> -

>  #endif /* __CONFIG_DRA7XX_EVM_H */

> 

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Roger Quadros Feb. 1, 2017, 9:42 a.m. | #2
On 31/01/17 06:17, Lokesh Vutla wrote:
> 

> 

> On Monday 30 January 2017 08:45 PM, Roger Quadros wrote:

>> In stead of defining the board EEPROM address in the board headers

>> let's define them in the board config files and make them

>> configurable by Kconfig.

>>

>> Signed-off-by: Roger Quadros <rogerq@ti.com>

>> ---

>>  arch/arm/Kconfig                  | 10 ++++++++++

>>  configs/am57xx_evm_defconfig      |  2 ++

>>  configs/am57xx_evm_nodt_defconfig |  2 ++

>>  configs/am57xx_hs_evm_defconfig   |  2 ++

>>  configs/dra7xx_evm_defconfig      |  2 ++

>>  configs/dra7xx_hs_evm_defconfig   |  2 ++

>>  include/configs/am57xx_evm.h      |  4 ----

>>  include/configs/dra7xx_evm.h      |  4 ----

>>  8 files changed, 20 insertions(+), 8 deletions(-)

>>

>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig

>> index c04adfb..19886ad 100644

>> --- a/arch/arm/Kconfig

>> +++ b/arch/arm/Kconfig

>> @@ -1009,6 +1009,16 @@ config TARGET_THUNDERX_88XX

>>  

>>  endchoice

>>  

>> +config EEPROM_BUS_ADDRESS

>> +        int "Board EEPROM's I2C bus address"

>> +        range 0 8

>> +        default 0

>> +

>> +config EEPROM_CHIP_ADDRESS

>> +        hex "Board EEPROM's I2C chip address"

>> +        range 0 0xff

>> +        default 0x50

>> +

> 

> IMO, these should be placed in board/ti/common/Kconfig. Also need not

> specify the value explicitly in defconfig if default value is used.


"board/ti/common/Kconfig" is not used by keystone
Is it OK to source it in board/ti/ks2_evm/Kconfig?

-- 
cheers,
-roger
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Lokesh Vutla Feb. 1, 2017, 10:47 a.m. | #3
On Wednesday 01 February 2017 03:12 PM, Roger Quadros wrote:
> On 31/01/17 06:17, Lokesh Vutla wrote:

>>

>>

>> On Monday 30 January 2017 08:45 PM, Roger Quadros wrote:

>>> In stead of defining the board EEPROM address in the board headers

>>> let's define them in the board config files and make them

>>> configurable by Kconfig.

>>>

>>> Signed-off-by: Roger Quadros <rogerq@ti.com>

>>> ---

>>>  arch/arm/Kconfig                  | 10 ++++++++++

>>>  configs/am57xx_evm_defconfig      |  2 ++

>>>  configs/am57xx_evm_nodt_defconfig |  2 ++

>>>  configs/am57xx_hs_evm_defconfig   |  2 ++

>>>  configs/dra7xx_evm_defconfig      |  2 ++

>>>  configs/dra7xx_hs_evm_defconfig   |  2 ++

>>>  include/configs/am57xx_evm.h      |  4 ----

>>>  include/configs/dra7xx_evm.h      |  4 ----

>>>  8 files changed, 20 insertions(+), 8 deletions(-)

>>>

>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig

>>> index c04adfb..19886ad 100644

>>> --- a/arch/arm/Kconfig

>>> +++ b/arch/arm/Kconfig

>>> @@ -1009,6 +1009,16 @@ config TARGET_THUNDERX_88XX

>>>  

>>>  endchoice

>>>  

>>> +config EEPROM_BUS_ADDRESS

>>> +        int "Board EEPROM's I2C bus address"

>>> +        range 0 8

>>> +        default 0

>>> +

>>> +config EEPROM_CHIP_ADDRESS

>>> +        hex "Board EEPROM's I2C chip address"

>>> +        range 0 0xff

>>> +        default 0x50

>>> +

>>

>> IMO, these should be placed in board/ti/common/Kconfig. Also need not

>> specify the value explicitly in defconfig if default value is used.

> 

> "board/ti/common/Kconfig" is not used by keystone

> Is it OK to source it in board/ti/ks2_evm/Kconfig?


yes. I don't see any problem with that :)

Thanks and regards,
Lokesh

> 

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Roger Quadros Feb. 1, 2017, 12:01 p.m. | #4
On 01/02/17 12:47, Lokesh Vutla wrote:
> 

> 

> On Wednesday 01 February 2017 03:12 PM, Roger Quadros wrote:

>> On 31/01/17 06:17, Lokesh Vutla wrote:

>>>

>>>

>>> On Monday 30 January 2017 08:45 PM, Roger Quadros wrote:

>>>> In stead of defining the board EEPROM address in the board headers

>>>> let's define them in the board config files and make them

>>>> configurable by Kconfig.

>>>>

>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>

>>>> ---

>>>>  arch/arm/Kconfig                  | 10 ++++++++++

>>>>  configs/am57xx_evm_defconfig      |  2 ++

>>>>  configs/am57xx_evm_nodt_defconfig |  2 ++

>>>>  configs/am57xx_hs_evm_defconfig   |  2 ++

>>>>  configs/dra7xx_evm_defconfig      |  2 ++

>>>>  configs/dra7xx_hs_evm_defconfig   |  2 ++

>>>>  include/configs/am57xx_evm.h      |  4 ----

>>>>  include/configs/dra7xx_evm.h      |  4 ----

>>>>  8 files changed, 20 insertions(+), 8 deletions(-)

>>>>

>>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig

>>>> index c04adfb..19886ad 100644

>>>> --- a/arch/arm/Kconfig

>>>> +++ b/arch/arm/Kconfig

>>>> @@ -1009,6 +1009,16 @@ config TARGET_THUNDERX_88XX

>>>>  

>>>>  endchoice

>>>>  

>>>> +config EEPROM_BUS_ADDRESS

>>>> +        int "Board EEPROM's I2C bus address"

>>>> +        range 0 8

>>>> +        default 0

>>>> +

>>>> +config EEPROM_CHIP_ADDRESS

>>>> +        hex "Board EEPROM's I2C chip address"

>>>> +        range 0 0xff

>>>> +        default 0x50

>>>> +

>>>

>>> IMO, these should be placed in board/ti/common/Kconfig. Also need not

>>> specify the value explicitly in defconfig if default value is used.

>>

>> "board/ti/common/Kconfig" is not used by keystone

>> Is it OK to source it in board/ti/ks2_evm/Kconfig?

> 

> yes. I don't see any problem with that :)

> 


If I do that I get the below error.
I think it is because SPL_NAND is being enabled but not required for K2?
Are all options in ti/common/Kconfig relevant for K2?

  CC      spl/common/spl/spl_nand.o
common/spl/spl_nand.c: In function 'spl_nand_load_image':
common/spl/spl_nand.c:139:41: error: 'CONFIG_SYS_NAND_U_BOOT_OFFS' undeclared (first use in this function)
  err = spl_nand_load_element(spl_image, CONFIG_SYS_NAND_U_BOOT_OFFS,
                                         ^
common/spl/spl_nand.c:139:41: note: each undeclared identifier is reported only once for each function it appears in
scripts/Makefile.build:280: recipe for target 'spl/common/spl/spl_nand.o' failed
make[2]: *** [spl/common/spl/spl_nand.o] Error 1
scripts/Makefile.spl:296: recipe for target 'spl/common/spl' failed
make[1]: *** [spl/common/spl] Error 2
Makefile:1330: recipe for target 'spl/u-boot-spl' failed
make: *** [spl/u-boot-spl] Error 2

-- 
cheers,
-roger
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Tom Rini Feb. 1, 2017, 12:14 p.m. | #5
On Wed, Feb 01, 2017 at 02:01:11PM +0200, Roger Quadros wrote:
> On 01/02/17 12:47, Lokesh Vutla wrote:

> > 

> > 

> > On Wednesday 01 February 2017 03:12 PM, Roger Quadros wrote:

> >> On 31/01/17 06:17, Lokesh Vutla wrote:

> >>>

> >>>

> >>> On Monday 30 January 2017 08:45 PM, Roger Quadros wrote:

> >>>> In stead of defining the board EEPROM address in the board headers

> >>>> let's define them in the board config files and make them

> >>>> configurable by Kconfig.

> >>>>

> >>>> Signed-off-by: Roger Quadros <rogerq@ti.com>

> >>>> ---

> >>>>  arch/arm/Kconfig                  | 10 ++++++++++

> >>>>  configs/am57xx_evm_defconfig      |  2 ++

> >>>>  configs/am57xx_evm_nodt_defconfig |  2 ++

> >>>>  configs/am57xx_hs_evm_defconfig   |  2 ++

> >>>>  configs/dra7xx_evm_defconfig      |  2 ++

> >>>>  configs/dra7xx_hs_evm_defconfig   |  2 ++

> >>>>  include/configs/am57xx_evm.h      |  4 ----

> >>>>  include/configs/dra7xx_evm.h      |  4 ----

> >>>>  8 files changed, 20 insertions(+), 8 deletions(-)

> >>>>

> >>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig

> >>>> index c04adfb..19886ad 100644

> >>>> --- a/arch/arm/Kconfig

> >>>> +++ b/arch/arm/Kconfig

> >>>> @@ -1009,6 +1009,16 @@ config TARGET_THUNDERX_88XX

> >>>>  

> >>>>  endchoice

> >>>>  

> >>>> +config EEPROM_BUS_ADDRESS

> >>>> +        int "Board EEPROM's I2C bus address"

> >>>> +        range 0 8

> >>>> +        default 0

> >>>> +

> >>>> +config EEPROM_CHIP_ADDRESS

> >>>> +        hex "Board EEPROM's I2C chip address"

> >>>> +        range 0 0xff

> >>>> +        default 0x50

> >>>> +

> >>>

> >>> IMO, these should be placed in board/ti/common/Kconfig. Also need not

> >>> specify the value explicitly in defconfig if default value is used.

> >>

> >> "board/ti/common/Kconfig" is not used by keystone

> >> Is it OK to source it in board/ti/ks2_evm/Kconfig?

> > 

> > yes. I don't see any problem with that :)

> > 

> 

> If I do that I get the below error.

> I think it is because SPL_NAND is being enabled but not required for K2?

> Are all options in ti/common/Kconfig relevant for K2?


The Kconfig file may need some minor tweaks / dependencies added to be
used on keystone, and long term we'll be doing all of those "config
SPL_FOO\ndefault y" entries another way.  The NAND entry could be
changed (but you'll need to test, and shoving a test branch at travis-ci
is an easy way to test) to be instead of 'default y' be 'default
CMD_NAND' I think.

-- 
Tom
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Roger Quadros Feb. 1, 2017, 1:05 p.m. | #6
On 01/02/17 14:14, Tom Rini wrote:
> On Wed, Feb 01, 2017 at 02:01:11PM +0200, Roger Quadros wrote:

>> On 01/02/17 12:47, Lokesh Vutla wrote:

>>>

>>>

>>> On Wednesday 01 February 2017 03:12 PM, Roger Quadros wrote:

>>>> On 31/01/17 06:17, Lokesh Vutla wrote:

>>>>>

>>>>>

>>>>> On Monday 30 January 2017 08:45 PM, Roger Quadros wrote:

>>>>>> In stead of defining the board EEPROM address in the board headers

>>>>>> let's define them in the board config files and make them

>>>>>> configurable by Kconfig.

>>>>>>

>>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>

>>>>>> ---

>>>>>>  arch/arm/Kconfig                  | 10 ++++++++++

>>>>>>  configs/am57xx_evm_defconfig      |  2 ++

>>>>>>  configs/am57xx_evm_nodt_defconfig |  2 ++

>>>>>>  configs/am57xx_hs_evm_defconfig   |  2 ++

>>>>>>  configs/dra7xx_evm_defconfig      |  2 ++

>>>>>>  configs/dra7xx_hs_evm_defconfig   |  2 ++

>>>>>>  include/configs/am57xx_evm.h      |  4 ----

>>>>>>  include/configs/dra7xx_evm.h      |  4 ----

>>>>>>  8 files changed, 20 insertions(+), 8 deletions(-)

>>>>>>

>>>>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig

>>>>>> index c04adfb..19886ad 100644

>>>>>> --- a/arch/arm/Kconfig

>>>>>> +++ b/arch/arm/Kconfig

>>>>>> @@ -1009,6 +1009,16 @@ config TARGET_THUNDERX_88XX

>>>>>>  

>>>>>>  endchoice

>>>>>>  

>>>>>> +config EEPROM_BUS_ADDRESS

>>>>>> +        int "Board EEPROM's I2C bus address"

>>>>>> +        range 0 8

>>>>>> +        default 0

>>>>>> +

>>>>>> +config EEPROM_CHIP_ADDRESS

>>>>>> +        hex "Board EEPROM's I2C chip address"

>>>>>> +        range 0 0xff

>>>>>> +        default 0x50

>>>>>> +

>>>>>

>>>>> IMO, these should be placed in board/ti/common/Kconfig. Also need not

>>>>> specify the value explicitly in defconfig if default value is used.

>>>>

>>>> "board/ti/common/Kconfig" is not used by keystone

>>>> Is it OK to source it in board/ti/ks2_evm/Kconfig?

>>>

>>> yes. I don't see any problem with that :)

>>>

>>

>> If I do that I get the below error.

>> I think it is because SPL_NAND is being enabled but not required for K2?

>> Are all options in ti/common/Kconfig relevant for K2?

> 

> The Kconfig file may need some minor tweaks / dependencies added to be

> used on keystone, and long term we'll be doing all of those "config

> SPL_FOO\ndefault y" entries another way.  The NAND entry could be

> changed (but you'll need to test, and shoving a test branch at travis-ci

> is an easy way to test) to be instead of 'default y' be 'default

> CMD_NAND' I think.

> 


It starts to look ugly if I add conditions in the Kconfig file.
Why are we setting default y to various SPL_* options in this file?
Shouldn't the board config file explicitly set the values?
e.g. some TI board might not need SPL_I2C and other might need it.

-- 
cheers,
-roger
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Tom Rini Feb. 1, 2017, 1:44 p.m. | #7
On Wed, Feb 01, 2017 at 03:05:18PM +0200, Roger Quadros wrote:
> On 01/02/17 14:14, Tom Rini wrote:

> > On Wed, Feb 01, 2017 at 02:01:11PM +0200, Roger Quadros wrote:

> >> On 01/02/17 12:47, Lokesh Vutla wrote:

> >>>

> >>>

> >>> On Wednesday 01 February 2017 03:12 PM, Roger Quadros wrote:

> >>>> On 31/01/17 06:17, Lokesh Vutla wrote:

> >>>>>

> >>>>>

> >>>>> On Monday 30 January 2017 08:45 PM, Roger Quadros wrote:

> >>>>>> In stead of defining the board EEPROM address in the board headers

> >>>>>> let's define them in the board config files and make them

> >>>>>> configurable by Kconfig.

> >>>>>>

> >>>>>> Signed-off-by: Roger Quadros <rogerq@ti.com>

> >>>>>> ---

> >>>>>>  arch/arm/Kconfig                  | 10 ++++++++++

> >>>>>>  configs/am57xx_evm_defconfig      |  2 ++

> >>>>>>  configs/am57xx_evm_nodt_defconfig |  2 ++

> >>>>>>  configs/am57xx_hs_evm_defconfig   |  2 ++

> >>>>>>  configs/dra7xx_evm_defconfig      |  2 ++

> >>>>>>  configs/dra7xx_hs_evm_defconfig   |  2 ++

> >>>>>>  include/configs/am57xx_evm.h      |  4 ----

> >>>>>>  include/configs/dra7xx_evm.h      |  4 ----

> >>>>>>  8 files changed, 20 insertions(+), 8 deletions(-)

> >>>>>>

> >>>>>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig

> >>>>>> index c04adfb..19886ad 100644

> >>>>>> --- a/arch/arm/Kconfig

> >>>>>> +++ b/arch/arm/Kconfig

> >>>>>> @@ -1009,6 +1009,16 @@ config TARGET_THUNDERX_88XX

> >>>>>>  

> >>>>>>  endchoice

> >>>>>>  

> >>>>>> +config EEPROM_BUS_ADDRESS

> >>>>>> +        int "Board EEPROM's I2C bus address"

> >>>>>> +        range 0 8

> >>>>>> +        default 0

> >>>>>> +

> >>>>>> +config EEPROM_CHIP_ADDRESS

> >>>>>> +        hex "Board EEPROM's I2C chip address"

> >>>>>> +        range 0 0xff

> >>>>>> +        default 0x50

> >>>>>> +

> >>>>>

> >>>>> IMO, these should be placed in board/ti/common/Kconfig. Also need not

> >>>>> specify the value explicitly in defconfig if default value is used.

> >>>>

> >>>> "board/ti/common/Kconfig" is not used by keystone

> >>>> Is it OK to source it in board/ti/ks2_evm/Kconfig?

> >>>

> >>> yes. I don't see any problem with that :)

> >>>

> >>

> >> If I do that I get the below error.

> >> I think it is because SPL_NAND is being enabled but not required for K2?

> >> Are all options in ti/common/Kconfig relevant for K2?

> > 

> > The Kconfig file may need some minor tweaks / dependencies added to be

> > used on keystone, and long term we'll be doing all of those "config

> > SPL_FOO\ndefault y" entries another way.  The NAND entry could be

> > changed (but you'll need to test, and shoving a test branch at travis-ci

> > is an easy way to test) to be instead of 'default y' be 'default

> > CMD_NAND' I think.

> 

> It starts to look ugly if I add conditions in the Kconfig file.

> Why are we setting default y to various SPL_* options in this file?

> Shouldn't the board config file explicitly set the values?

> e.g. some TI board might not need SPL_I2C and other might need it.


Until the 'imply' keyword comes in to Kconfig we're left with a few
different ugly options in order to avoid having to make the defconfig
files themselves unwieldy.  A 'default y' does not make it impossible to
disable the feature, but changing the default here potentially easier
than setting it to N in the various k2 defconfig files, for example.

-- 
Tom
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c04adfb..19886ad 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1009,6 +1009,16 @@  config TARGET_THUNDERX_88XX
 
 endchoice
 
+config EEPROM_BUS_ADDRESS
+        int "Board EEPROM's I2C bus address"
+        range 0 8
+        default 0
+
+config EEPROM_CHIP_ADDRESS
+        hex "Board EEPROM's I2C chip address"
+        range 0 0xff
+        default 0x50
+
 source "arch/arm/mach-at91/Kconfig"
 
 source "arch/arm/mach-bcm283x/Kconfig"
diff --git a/configs/am57xx_evm_defconfig b/configs/am57xx_evm_defconfig
index 656d991..3b3e74a 100644
--- a/configs/am57xx_evm_defconfig
+++ b/configs/am57xx_evm_defconfig
@@ -2,6 +2,8 @@  CONFIG_ARM=y
 CONFIG_OMAP54XX=y
 # CONFIG_SPL_NAND_SUPPORT is not set
 CONFIG_TARGET_AM57XX_EVM=y
+CONFIG_EEPROM_BUS_ADDRESS=0
+CONFIG_EEPROM_CHIP_ADDRESS=0x50
 CONFIG_SPL_SPI_FLASH_SUPPORT=y
 CONFIG_SPL_SPI_SUPPORT=y
 CONFIG_ARMV7_LPAE=y
diff --git a/configs/am57xx_evm_nodt_defconfig b/configs/am57xx_evm_nodt_defconfig
index b3b95f9..8d722e7 100644
--- a/configs/am57xx_evm_nodt_defconfig
+++ b/configs/am57xx_evm_nodt_defconfig
@@ -2,6 +2,8 @@  CONFIG_ARM=y
 CONFIG_OMAP54XX=y
 # CONFIG_SPL_NAND_SUPPORT is not set
 CONFIG_TARGET_AM57XX_EVM=y
+CONFIG_EEPROM_BUS_ADDRESS=0
+CONFIG_EEPROM_CHIP_ADDRESS=0x50
 CONFIG_SPL_SPI_FLASH_SUPPORT=y
 CONFIG_SPL_SPI_SUPPORT=y
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
diff --git a/configs/am57xx_hs_evm_defconfig b/configs/am57xx_hs_evm_defconfig
index d920d68..36c8004 100644
--- a/configs/am57xx_hs_evm_defconfig
+++ b/configs/am57xx_hs_evm_defconfig
@@ -3,6 +3,8 @@  CONFIG_OMAP54XX=y
 CONFIG_TI_SECURE_DEVICE=y
 # CONFIG_SPL_NAND_SUPPORT is not set
 CONFIG_TARGET_AM57XX_EVM=y
+CONFIG_EEPROM_BUS_ADDRESS=0
+CONFIG_EEPROM_CHIP_ADDRESS=0x50
 CONFIG_TI_SECURE_EMIF_REGION_START=0xbe000000
 CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE=0x02000000
 CONFIG_TI_SECURE_EMIF_PROTECTED_REGION_SIZE=0x01c00000
diff --git a/configs/dra7xx_evm_defconfig b/configs/dra7xx_evm_defconfig
index 1836021..3108c58 100644
--- a/configs/dra7xx_evm_defconfig
+++ b/configs/dra7xx_evm_defconfig
@@ -2,6 +2,8 @@  CONFIG_ARM=y
 CONFIG_OMAP54XX=y
 # CONFIG_SPL_NAND_SUPPORT is not set
 CONFIG_TARGET_DRA7XX_EVM=y
+CONFIG_EEPROM_BUS_ADDRESS=0
+CONFIG_EEPROM_CHIP_ADDRESS=0x50
 CONFIG_SPL_SPI_FLASH_SUPPORT=y
 CONFIG_SPL_SPI_SUPPORT=y
 CONFIG_ARMV7_LPAE=y
diff --git a/configs/dra7xx_hs_evm_defconfig b/configs/dra7xx_hs_evm_defconfig
index 1d107e4..7f5ce49 100644
--- a/configs/dra7xx_hs_evm_defconfig
+++ b/configs/dra7xx_hs_evm_defconfig
@@ -4,6 +4,8 @@  CONFIG_SYS_MALLOC_F_LEN=0x2000
 CONFIG_TI_SECURE_DEVICE=y
 # CONFIG_SPL_NAND_SUPPORT is not set
 CONFIG_TARGET_DRA7XX_EVM=y
+CONFIG_EEPROM_BUS_ADDRESS=0
+CONFIG_EEPROM_CHIP_ADDRESS=0x50
 CONFIG_TI_SECURE_EMIF_REGION_START=0xbe000000
 CONFIG_TI_SECURE_EMIF_TOTAL_REGION_SIZE=0x02000000
 CONFIG_TI_SECURE_EMIF_PROTECTED_REGION_SIZE=0x01c00000
diff --git a/include/configs/am57xx_evm.h b/include/configs/am57xx_evm.h
index 840502c..d9e1119 100644
--- a/include/configs/am57xx_evm.h
+++ b/include/configs/am57xx_evm.h
@@ -105,10 +105,6 @@ 
 #define CONFIG_SYS_SCSI_MAX_DEVICE	(CONFIG_SYS_SCSI_MAX_SCSI_ID * \
 						CONFIG_SYS_SCSI_MAX_LUN)
 
-/* EEPROM */
-#define CONFIG_EEPROM_CHIP_ADDRESS 0x50
-#define CONFIG_EEPROM_BUS_ADDRESS 0
-
 /*
  * Default to using SPI for environment, etc.
  * 0x000000 - 0x040000 : QSPI.SPL (256KiB)
diff --git a/include/configs/dra7xx_evm.h b/include/configs/dra7xx_evm.h
index da458a4..46beb8b 100644
--- a/include/configs/dra7xx_evm.h
+++ b/include/configs/dra7xx_evm.h
@@ -264,8 +264,4 @@ 
 #endif
 #endif  /* NOR support */
 
-/* EEPROM */
-#define CONFIG_EEPROM_CHIP_ADDRESS 0x50
-#define CONFIG_EEPROM_BUS_ADDRESS 0
-
 #endif /* __CONFIG_DRA7XX_EVM_H */