Re: [U-Boot] [PATCH v2 5/5] board: ti: AM57xx: Add detection logic for AM57xx-evm

2015-11-03 Thread Steven Kipisz

On 11/03/2015 07:29 AM, Igor Grinberg wrote:

Hi Steve,

On 11/03/15 14:22, Steve Kipisz wrote:

[...]


Signed-off-by: Steve Kipisz 
---
v2 Based on:
  master a6104737 ARM: at91: sama5: change the environment address to 0x6000

Build testing: MAKEALL -s omap4 -s omap5 (no warnings/build errors)
Boot Testing:
am57xx_evm_nodt_config: http://pastebin.ubuntu.com/13039296/
beagle_x15_config: http://pastebin.ubuntu.com/13039331/

Changes in v2 (since v1):
- move the board detection code into the new routine
  do_board_detect
- eliminate board.h and move the ix_xxx into board.c
- redo commit message to be more clear

v1:  http://marc.info/?t=14460800792=1=2
  http://marc.info/?t=14460800794=1=2
(mailing list squashed original submission)


[...]


+#define is_x15()   board_am_is("BBRDX15_")
+#define is_am572x_evm()board_am_is("AM572PM_")


I think board_is_* much more appropriate here...


Ok. so board_is_x15 and board_is_am572x_evm

+
  #ifdef CONFIG_DRIVER_TI_CPSW
  #include 
  #endif
@@ -246,6 +249,54 @@ struct vcores_data beagle_x15_volts = {
.iva.pmic   = ,
  };

+#ifdef CONFIG_SPL_BUILD
+/* No env to setup for SPL */
+static inline void setup_board_eeprom_env(void) { }
+
+/* Override function to read eeprom information */
+void do_board_detect(void)
+{
+   struct ti_am_eeprom *ep;
+   int rc;
+
+   rc = ti_i2c_eeprom_am_get(CONFIG_EEPROM_BUS_ADDRESS,
+ CONFIG_EEPROM_CHIP_ADDRESS, );
+   if (rc)
+   printf("ti_i2c_eeprom_init failed %d\n", rc);
+}


Do you really need this in SPL?


Yes. We need to detect the board to determine DDR setup, pin mux, 
iodelay. All of that needs to be done in SPL. X15 and EVM are the same, 
but more boards will be added that have some differences.



+
+#else  /* CONFIG_SPL_BUILD */
+
+static void setup_board_eeprom_env(void)
+{
+   char *name = NULL;


How about:

char *name = "beagle_x15";


+   int rc;
+   struct ti_am_eeprom_printable p;
+
+   rc = ti_i2c_eeprom_am_get_print(CONFIG_EEPROM_BUS_ADDRESS,
+   CONFIG_EEPROM_CHIP_ADDRESS, );
+   if (rc) {
+   printf("Invalid EEPROM data(@0x%p). Default to X15\n",
+  TI_AM_EEPROM_DATA);
+   goto invalid_eeprom;
+   }
+
+   if (is_x15())
+   name = "beagle_x15";


This will not be needed if the above comment is implemented.


+   else if (is_am572x_evm())
+   name = "am57xx_evm";
+   else
+   printf("Unidentified board claims %s in eeprom header\n",
+  p.name);
+
+invalid_eeprom:
+   set_board_info_env(name, "beagle_x15", p.version, p.serial);


If the above comment is implemented, no more need for the
default_name parameter...


Ok, I'll look at that.

+}
+
+/* Eeprom is alread read by SPL.. nothing more to do here.. */
+
+#endif /* CONFIG_SPL_BUILD */


[...]



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


Re: [U-Boot] [PATCH v2 5/5] board: ti: AM57xx: Add detection logic for AM57xx-evm

2015-11-03 Thread Igor Grinberg
Hi Steve,

On 11/03/15 14:22, Steve Kipisz wrote:

[...]

> Signed-off-by: Steve Kipisz 
> ---
> v2 Based on:
>  master a6104737 ARM: at91: sama5: change the environment address to 
> 0x6000
> 
> Build testing: MAKEALL -s omap4 -s omap5 (no warnings/build errors)
>   Boot Testing:
>   am57xx_evm_nodt_config: http://pastebin.ubuntu.com/13039296/
>   beagle_x15_config: http://pastebin.ubuntu.com/13039331/
> 
> Changes in v2 (since v1):
>   - move the board detection code into the new routine
> do_board_detect
>   - eliminate board.h and move the ix_xxx into board.c
>   - redo commit message to be more clear
> 
> v1:  http://marc.info/?t=14460800792=1=2
>  http://marc.info/?t=14460800794=1=2
>   (mailing list squashed original submission)

[...]

> +#define is_x15() board_am_is("BBRDX15_")
> +#define is_am572x_evm()  board_am_is("AM572PM_")

I think board_is_* much more appropriate here...

> +
>  #ifdef CONFIG_DRIVER_TI_CPSW
>  #include 
>  #endif
> @@ -246,6 +249,54 @@ struct vcores_data beagle_x15_volts = {
>   .iva.pmic   = ,
>  };
>  
> +#ifdef CONFIG_SPL_BUILD
> +/* No env to setup for SPL */
> +static inline void setup_board_eeprom_env(void) { }
> +
> +/* Override function to read eeprom information */
> +void do_board_detect(void)
> +{
> + struct ti_am_eeprom *ep;
> + int rc;
> +
> + rc = ti_i2c_eeprom_am_get(CONFIG_EEPROM_BUS_ADDRESS,
> +   CONFIG_EEPROM_CHIP_ADDRESS, );
> + if (rc)
> + printf("ti_i2c_eeprom_init failed %d\n", rc);
> +}

Do you really need this in SPL?

> +
> +#else/* CONFIG_SPL_BUILD */
> +
> +static void setup_board_eeprom_env(void)
> +{
> + char *name = NULL;

How about:

char *name = "beagle_x15";

> + int rc;
> + struct ti_am_eeprom_printable p;
> +
> + rc = ti_i2c_eeprom_am_get_print(CONFIG_EEPROM_BUS_ADDRESS,
> + CONFIG_EEPROM_CHIP_ADDRESS, );
> + if (rc) {
> + printf("Invalid EEPROM data(@0x%p). Default to X15\n",
> +TI_AM_EEPROM_DATA);
> + goto invalid_eeprom;
> + }
> +
> + if (is_x15())
> + name = "beagle_x15";

This will not be needed if the above comment is implemented.

> + else if (is_am572x_evm())
> + name = "am57xx_evm";
> + else
> + printf("Unidentified board claims %s in eeprom header\n",
> +p.name);
> +
> +invalid_eeprom:
> + set_board_info_env(name, "beagle_x15", p.version, p.serial);

If the above comment is implemented, no more need for the
default_name parameter...

> +}
> +
> +/* Eeprom is alread read by SPL.. nothing more to do here.. */
> +
> +#endif   /* CONFIG_SPL_BUILD */

[...]


-- 
Regards,
Igor.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH v2 5/5] board: ti: AM57xx: Add detection logic for AM57xx-evm

2015-11-03 Thread Steve Kipisz
Current AM57xx evm supports both BeagleBoard-X15
(http://beagleboard.org/x15) and AM57xx EVM
(http://www.ti.com/tool/tmdxevm5728).

The AM572x EValuation Module(EVM) provides an affordable platform to
quickly start evaluation of Sitara. ARM Cortex-A15 AM57x Processors
(AM5728, AM5726, AM5718, AM5716) and accelerate development for HMI,
machine vision, networking, medical imaging and many other industrial
applications. This EVM is based on the same BeagleBoard-X15 Chassis
and adds mPCIe, mSATA, LCD, touchscreen, Camera, push button and TI's
wlink8 offering.

Since the EEPROM contents are compatible between the BeagleBoard-X15 and
the AM57xx-evm, we add support for the detection logic to enable
support for various user programmable scripting capability.

NOTE: U-boot configuration is currently a superset of AM57xx evm and
BeagleBoard-X15 and no additional configuration tweaking is needed.

This change also sets up the stage for future support of TI AM57xx EVMs
to the same base bootloader build.

Signed-off-by: Steve Kipisz 
---
v2 Based on:
 master a6104737 ARM: at91: sama5: change the environment address to 0x6000

Build testing: MAKEALL -s omap4 -s omap5 (no warnings/build errors)
Boot Testing:
am57xx_evm_nodt_config: http://pastebin.ubuntu.com/13039296/
beagle_x15_config: http://pastebin.ubuntu.com/13039331/

Changes in v2 (since v1):
- move the board detection code into the new routine
  do_board_detect
- eliminate board.h and move the ix_xxx into board.c
- redo commit message to be more clear

v1:  http://marc.info/?t=14460800792=1=2
 http://marc.info/?t=14460800794=1=2
(mailing list squashed original submission)

 board/ti/am57xx/board.c   | 53 +++
 include/configs/am57xx_evm.h  |  4 +++
 include/configs/ti_omap5_common.h |  2 ++
 3 files changed, 59 insertions(+)

diff --git a/board/ti/am57xx/board.c b/board/ti/am57xx/board.c
index 042f9ab1965a..41dd8333b1eb 100644
--- a/board/ti/am57xx/board.c
+++ b/board/ti/am57xx/board.c
@@ -32,6 +32,9 @@
 
 #include "mux_data.h"
 
+#define is_x15()   board_am_is("BBRDX15_")
+#define is_am572x_evm()board_am_is("AM572PM_")
+
 #ifdef CONFIG_DRIVER_TI_CPSW
 #include 
 #endif
@@ -246,6 +249,54 @@ struct vcores_data beagle_x15_volts = {
.iva.pmic   = ,
 };
 
+#ifdef CONFIG_SPL_BUILD
+/* No env to setup for SPL */
+static inline void setup_board_eeprom_env(void) { }
+
+/* Override function to read eeprom information */
+void do_board_detect(void)
+{
+   struct ti_am_eeprom *ep;
+   int rc;
+
+   rc = ti_i2c_eeprom_am_get(CONFIG_EEPROM_BUS_ADDRESS,
+ CONFIG_EEPROM_CHIP_ADDRESS, );
+   if (rc)
+   printf("ti_i2c_eeprom_init failed %d\n", rc);
+}
+
+#else  /* CONFIG_SPL_BUILD */
+
+static void setup_board_eeprom_env(void)
+{
+   char *name = NULL;
+   int rc;
+   struct ti_am_eeprom_printable p;
+
+   rc = ti_i2c_eeprom_am_get_print(CONFIG_EEPROM_BUS_ADDRESS,
+   CONFIG_EEPROM_CHIP_ADDRESS, );
+   if (rc) {
+   printf("Invalid EEPROM data(@0x%p). Default to X15\n",
+  TI_AM_EEPROM_DATA);
+   goto invalid_eeprom;
+   }
+
+   if (is_x15())
+   name = "beagle_x15";
+   else if (is_am572x_evm())
+   name = "am57xx_evm";
+   else
+   printf("Unidentified board claims %s in eeprom header\n",
+  p.name);
+
+invalid_eeprom:
+   set_board_info_env(name, "beagle_x15", p.version, p.serial);
+}
+
+/* Eeprom is alread read by SPL.. nothing more to do here.. */
+
+#endif /* CONFIG_SPL_BUILD */
+
 void hw_data_init(void)
 {
*prcm = _prcm;
@@ -265,6 +316,8 @@ int board_init(void)
 int board_late_init(void)
 {
init_sata(0);
+   setup_board_eeprom_env();
+
/*
 * DEV_CTRL.DEV_ON = 1 please - else palmas switches off in 8 seconds
 * This is the POWERHOLD-in-Low behavior.
diff --git a/include/configs/am57xx_evm.h b/include/configs/am57xx_evm.h
index 6308cab8e680..1fffdb18fbcd 100644
--- a/include/configs/am57xx_evm.h
+++ b/include/configs/am57xx_evm.h
@@ -88,4 +88,8 @@
 #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
+
 #endif /* __CONFIG_AM57XX_EVM_H */
diff --git a/include/configs/ti_omap5_common.h 
b/include/configs/ti_omap5_common.h
index 5acbc92c3f60..ae6e2a556a93 100644
--- a/include/configs/ti_omap5_common.h
+++ b/include/configs/ti_omap5_common.h
@@ -120,6 +120,8 @@
"setenv fdtfile dra72-evm.dtb; fi;" \
"if test $board_name = beagle_x15; then " \
"setenv fdtfile am57xx-beagle-x15.dtb; 

Re: [U-Boot] [PATCH v2 5/5] board: ti: AM57xx: Add detection logic for AM57xx-evm

2015-11-03 Thread Nishanth Menon
On 11/03/2015 06:22 AM, Steve Kipisz wrote:
> diff --git a/include/configs/ti_omap5_common.h 
> b/include/configs/ti_omap5_common.h
> index 5acbc92c3f60..ae6e2a556a93 100644
> --- a/include/configs/ti_omap5_common.h
> +++ b/include/configs/ti_omap5_common.h
> @@ -120,6 +120,8 @@
>   "setenv fdtfile dra72-evm.dtb; fi;" \
>   "if test $board_name = beagle_x15; then " \
>   "setenv fdtfile am57xx-beagle-x15.dtb; fi;" \
> + "if test $board_name = am57xx_evm; then " \
> + "setenv fdtfile am57xx-evm.dtb; fi;" \
>   "if test $fdtfile = undefined; then " \
>   "echo WARNING: Could not determine device tree to use; 
> fi; \0" \
>   "loadfdt=load mmc ${bootpart} ${fdtaddr} ${bootdir}/${fdtfile};\0" \

Hmmm I might keep this nugget out of upstream code for now.
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/
does not contain am57xx-evm.

it is possible that when we post patches upstream, it might become dt
overlay instead of evm dtb. I dont know about what will eventually
become in upstream. So, I suggest dropping the above change.



Also, in the future, could you cc beagleboard-x15
 for all patches that impact
beagleboard-X15 I am sure folks there will be interested to review as well?

-- 
Regards,
Nishanth Menon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 5/5] board: ti: AM57xx: Add detection logic for AM57xx-evm

2015-11-03 Thread Steven Kipisz

On 11/03/2015 09:31 AM, Nishanth Menon wrote:

On 11/03/2015 06:22 AM, Steve Kipisz wrote:

diff --git a/include/configs/ti_omap5_common.h 
b/include/configs/ti_omap5_common.h
index 5acbc92c3f60..ae6e2a556a93 100644
--- a/include/configs/ti_omap5_common.h
+++ b/include/configs/ti_omap5_common.h
@@ -120,6 +120,8 @@
"setenv fdtfile dra72-evm.dtb; fi;" \
"if test $board_name = beagle_x15; then " \
"setenv fdtfile am57xx-beagle-x15.dtb; fi;" \
+   "if test $board_name = am57xx_evm; then " \
+   "setenv fdtfile am57xx-evm.dtb; fi;" \
"if test $fdtfile = undefined; then " \
"echo WARNING: Could not determine device tree to use; fi; 
\0" \
"loadfdt=load mmc ${bootpart} ${fdtaddr} ${bootdir}/${fdtfile};\0" \


Hmmm I might keep this nugget out of upstream code for now.
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/
does not contain am57xx-evm.

it is possible that when we post patches upstream, it might become dt
overlay instead of evm dtb. I dont know about what will eventually
become in upstream. So, I suggest dropping the above change.



Got it.


Also, in the future, could you cc beagleboard-x15
 for all patches that impact
beagleboard-X15 I am sure folks there will be interested to review as well?


Ok. I'll add it to my send-email script.

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


Re: [U-Boot] [PATCH v2 5/5] board: ti: AM57xx: Add detection logic for AM57xx-evm

2015-11-03 Thread Igor Grinberg
On 11/03/15 17:09, Steven Kipisz wrote:
> On 11/03/2015 07:29 AM, Igor Grinberg wrote:
>> Hi Steve,
>>
>> On 11/03/15 14:22, Steve Kipisz wrote:
>>
>> [...]
>>
>>> Signed-off-by: Steve Kipisz 
>>> ---
>>> v2 Based on:
>>>   master a6104737 ARM: at91: sama5: change the environment address to 
>>> 0x6000
>>>
>>> Build testing: MAKEALL -s omap4 -s omap5 (no warnings/build errors)
>>> Boot Testing:
>>> am57xx_evm_nodt_config: http://pastebin.ubuntu.com/13039296/
>>> beagle_x15_config: http://pastebin.ubuntu.com/13039331/
>>>
>>> Changes in v2 (since v1):
>>> - move the board detection code into the new routine
>>>   do_board_detect
>>> - eliminate board.h and move the ix_xxx into board.c
>>> - redo commit message to be more clear
>>>
>>> v1:  http://marc.info/?t=14460800792=1=2
>>>   http://marc.info/?t=14460800794=1=2
>>> (mailing list squashed original submission)
>>
>> [...]
>>
>>> +#define is_x15()board_am_is("BBRDX15_")
>>> +#define is_am572x_evm()board_am_is("AM572PM_")
>>
>> I think board_is_* much more appropriate here...
>>
> Ok. so board_is_x15 and board_is_am572x_evm

Yep. Sounds better.

>>> +
>>>   #ifdef CONFIG_DRIVER_TI_CPSW
>>>   #include 
>>>   #endif
>>> @@ -246,6 +249,54 @@ struct vcores_data beagle_x15_volts = {
>>>   .iva.pmic= ,
>>>   };
>>>
>>> +#ifdef CONFIG_SPL_BUILD
>>> +/* No env to setup for SPL */
>>> +static inline void setup_board_eeprom_env(void) { }
>>> +
>>> +/* Override function to read eeprom information */
>>> +void do_board_detect(void)
>>> +{
>>> +struct ti_am_eeprom *ep;
>>> +int rc;
>>> +
>>> +rc = ti_i2c_eeprom_am_get(CONFIG_EEPROM_BUS_ADDRESS,
>>> +  CONFIG_EEPROM_CHIP_ADDRESS, );
>>> +if (rc)
>>> +printf("ti_i2c_eeprom_init failed %d\n", rc);
>>> +}
>>
>> Do you really need this in SPL?
> 
> Yes. We need to detect the board to determine DDR setup, pin mux, iodelay. 
> All of that needs to be done in SPL. X15 and EVM are the same, but more 
> boards will be added that have some differences.

Ok.

>>
>>> +
>>> +#else/* CONFIG_SPL_BUILD */
>>> +
>>> +static void setup_board_eeprom_env(void)
>>> +{
>>> +char *name = NULL;
>>
>> How about:
>>
>> char *name = "beagle_x15";
>>
>>> +int rc;
>>> +struct ti_am_eeprom_printable p;
>>> +
>>> +rc = ti_i2c_eeprom_am_get_print(CONFIG_EEPROM_BUS_ADDRESS,
>>> +CONFIG_EEPROM_CHIP_ADDRESS, );
>>> +if (rc) {
>>> +printf("Invalid EEPROM data(@0x%p). Default to X15\n",
>>> +   TI_AM_EEPROM_DATA);
>>> +goto invalid_eeprom;
>>> +}
>>> +
>>> +if (is_x15())
>>> +name = "beagle_x15";
>>
>> This will not be needed if the above comment is implemented.
>>
>>> +else if (is_am572x_evm())
>>> +name = "am57xx_evm";
>>> +else
>>> +printf("Unidentified board claims %s in eeprom header\n",
>>> +   p.name);
>>> +
>>> +invalid_eeprom:
>>> +set_board_info_env(name, "beagle_x15", p.version, p.serial);
>>
>> If the above comment is implemented, no more need for the
>> default_name parameter...
>>
> Ok, I'll look at that.
>>> +}
>>> +
>>> +/* Eeprom is alread read by SPL.. nothing more to do here.. */
>>> +
>>> +#endif/* CONFIG_SPL_BUILD */
>>
>> [...]
>>
>>
> Steve K.
> 

-- 
Regards,
Igor.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot