[U-Boot] [PATCH] PATI: fix broken SPI access

fix broken SPI access by adding/activating BOARD_EARLY_INIT_F functionality and calling spi_init_f() from there.
Signed-off-by: David Müller d.mueller@elsoft.ch --- board/mpl/pati/pati.c | 5 +++++ include/configs/PATI.h | 1 + 2 files changed, 6 insertions(+)
diff --git a/board/mpl/pati/pati.c b/board/mpl/pati/pati.c index 5d701a7..b9d88ee 100644 --- a/board/mpl/pati/pati.c +++ b/board/mpl/pati/pati.c @@ -311,6 +311,11 @@ void user_led1(int led_on) sysconf->sc_sgpiodt2=reg; /* Data register */ }
+int board_early_init_f(void) +{ + spi_init_f(); + return 0; +}
/**************************************************************** * Last Stage Init diff --git a/include/configs/PATI.h b/include/configs/PATI.h index 65ab65d..3ca204e 100644 --- a/include/configs/PATI.h +++ b/include/configs/PATI.h @@ -98,6 +98,7 @@
#define CONFIG_SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, 1250000 }
+#define CONFIG_BOARD_EARLY_INIT_F
/*********************************************************************** * Last Stage Init

On 30 September 2014 16:53, David Müller d.mueller@elsoft.ch wrote:
fix broken SPI access by adding/activating BOARD_EARLY_INIT_F functionality and calling spi_init_f() from there.
Signed-off-by: David Müller d.mueller@elsoft.ch
board/mpl/pati/pati.c | 5 +++++ include/configs/PATI.h | 1 + 2 files changed, 6 insertions(+)
diff --git a/board/mpl/pati/pati.c b/board/mpl/pati/pati.c index 5d701a7..b9d88ee 100644 --- a/board/mpl/pati/pati.c +++ b/board/mpl/pati/pati.c @@ -311,6 +311,11 @@ void user_led1(int led_on) sysconf->sc_sgpiodt2=reg; /* Data register */ }
+int board_early_init_f(void) +{
spi_init_f();
Why you need to do this, spi_init_f is trying to call from arch/powerpc/lib/board.c any specific reason, I couldn't understand the fix you mentioned on the commit body.
return 0;
+}
/****************************************************************
- Last Stage Init
diff --git a/include/configs/PATI.h b/include/configs/PATI.h index 65ab65d..3ca204e 100644 --- a/include/configs/PATI.h +++ b/include/configs/PATI.h @@ -98,6 +98,7 @@
#define CONFIG_SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, 1250000 }
+#define CONFIG_BOARD_EARLY_INIT_F
/***********************************************************************
- Last Stage Init
--
thanks!

Jagan Teki wrote:
On 30 September 2014 16:53, David Müller d.mueller@elsoft.ch wrote:
+int board_early_init_f(void) +{
spi_init_f();
Why you need to do this, spi_init_f is trying to call from arch/powerpc/lib/board.c any specific reason, I couldn't understand the fix you mentioned on the commit body.
There is an EEPROM attached to the SPI channel containing vital board data. Calling spi_init_f() from arch/powerpc/lib/board.c will be too late.

On 30 September 2014 18:41, David Müller (ELSOFT AG) d.mueller@elsoft.ch wrote:
Jagan Teki wrote:
On 30 September 2014 16:53, David Müller d.mueller@elsoft.ch wrote:
+int board_early_init_f(void) +{
spi_init_f();
Why you need to do this, spi_init_f is trying to call from arch/powerpc/lib/board.c any specific reason, I couldn't understand the fix you mentioned on the commit body.
There is an EEPROM attached to the SPI channel containing vital board data. Calling spi_init_f() from arch/powerpc/lib/board.c will be too late.
Sorry, this looks an other issue - but anyway we're trying to remove spi_init* stuff from drivers/spi/* in future and I don't think it's a good idea to use that.
thanks!

On Tue, Sep 30, 2014 at 08:06:43PM +0530, Jagan Teki wrote:
On 30 September 2014 18:41, David Müller (ELSOFT AG) d.mueller@elsoft.ch wrote:
Jagan Teki wrote:
On 30 September 2014 16:53, David Müller d.mueller@elsoft.ch wrote:
+int board_early_init_f(void) +{
spi_init_f();
Why you need to do this, spi_init_f is trying to call from arch/powerpc/lib/board.c any specific reason, I couldn't understand the fix you mentioned on the commit body.
There is an EEPROM attached to the SPI channel containing vital board data. Calling spi_init_f() from arch/powerpc/lib/board.c will be too late.
Sorry, this looks an other issue - but anyway we're trying to remove spi_init* stuff from drivers/spi/* in future and I don't think it's a good idea to use that.
It's also not a good idea to say that we'll leave a board broken until something better comes along. There should be a comment added to the code here making it clear _why_ we need this done early.

On Tue, Sep 30, 2014 at 10:48:11AM -0400, Tom Rini wrote:
On Tue, Sep 30, 2014 at 08:06:43PM +0530, Jagan Teki wrote:
On 30 September 2014 18:41, David Müller (ELSOFT AG) d.mueller@elsoft.ch wrote:
Jagan Teki wrote:
On 30 September 2014 16:53, David Müller d.mueller@elsoft.ch wrote:
+int board_early_init_f(void) +{
spi_init_f();
Why you need to do this, spi_init_f is trying to call from arch/powerpc/lib/board.c any specific reason, I couldn't understand the fix you mentioned on the commit body.
There is an EEPROM attached to the SPI channel containing vital board data. Calling spi_init_f() from arch/powerpc/lib/board.c will be too late.
Sorry, this looks an other issue - but anyway we're trying to remove spi_init* stuff from drivers/spi/* in future and I don't think it's a good idea to use that.
It's also not a good idea to say that we'll leave a board broken until something better comes along. There should be a comment added to the code here making it clear _why_ we need this done early.
And again, for the record, I wanted to _fix_ things today so we can clean them up tomorrow, rather than keep something broken so we can fix it later.

On 10 October 2014 21:44, Tom Rini trini@ti.com wrote:
On Tue, Sep 30, 2014 at 10:48:11AM -0400, Tom Rini wrote:
On Tue, Sep 30, 2014 at 08:06:43PM +0530, Jagan Teki wrote:
On 30 September 2014 18:41, David Müller (ELSOFT AG) d.mueller@elsoft.ch wrote:
Jagan Teki wrote:
On 30 September 2014 16:53, David Müller d.mueller@elsoft.ch wrote:
+int board_early_init_f(void) +{
spi_init_f();
Why you need to do this, spi_init_f is trying to call from arch/powerpc/lib/board.c any specific reason, I couldn't understand the fix you mentioned on the commit body.
There is an EEPROM attached to the SPI channel containing vital board data. Calling spi_init_f() from arch/powerpc/lib/board.c will be too late.
Sorry, this looks an other issue - but anyway we're trying to remove spi_init* stuff from drivers/spi/* in future and I don't think it's a good idea to use that.
It's also not a good idea to say that we'll leave a board broken until something better comes along. There should be a comment added to the code here making it clear _why_ we need this done early.
And again, for the record, I wanted to _fix_ things today so we can clean them up tomorrow, rather than keep something broken so we can fix it later.
OK, got your point.
thanks!

Jagan Teki wrote:
On 30 September 2014 18:41, David Müller (ELSOFT AG) d.mueller@elsoft.ch wrote:
Jagan Teki wrote:
On 30 September 2014 16:53, David Müller d.mueller@elsoft.ch wrote:
+int board_early_init_f(void) +{
spi_init_f();
Why you need to do this, spi_init_f is trying to call from arch/powerpc/lib/board.c any specific reason, I couldn't understand the fix you mentioned on the commit body.
There is an EEPROM attached to the SPI channel containing vital board data. Calling spi_init_f() from arch/powerpc/lib/board.c will be too late.
Sorry, this looks an other issue - but anyway we're trying to remove
What kind of "other issue" do you mean?
spi_init* stuff from drivers/spi/* in future and I don't think it's a good idea to use that.
In this case what is the proposed way of initializing the SPI subsystem?

On 30 September 2014 20:28, David Müller (ELSOFT AG) d.mueller@elsoft.ch wrote:
Jagan Teki wrote:
On 30 September 2014 18:41, David Müller (ELSOFT AG) d.mueller@elsoft.ch wrote:
Jagan Teki wrote:
On 30 September 2014 16:53, David Müller d.mueller@elsoft.ch wrote:
+int board_early_init_f(void) +{
spi_init_f();
Why you need to do this, spi_init_f is trying to call from arch/powerpc/lib/board.c any specific reason, I couldn't understand the fix you mentioned on the commit body.
There is an EEPROM attached to the SPI channel containing vital board data. Calling spi_init_f() from arch/powerpc/lib/board.c will be too late.
Sorry, this looks an other issue - but anyway we're trying to remove
What kind of "other issue" do you mean?
spi_init* stuff from drivers/spi/* in future and I don't think it's a good idea to use that.
In this case what is the proposed way of initializing the SPI subsystem?
Usually spi got initialized through setup_slave() from the cmd interface which is of dynamic probe ie the reason most of drivers have spi_init() with no functionality code.
On the other hand few are using spi_init() or spi_init_f() from arch code or from any other kind to initialize the spi.
Finally spi_init_f() from your case should use from arch(at least) as this call is more generic and it shouldn't be a call from board.
And more over we're working on dm-spi where spi_init() calls stuff would disappear in future.
thanks!

On Tue, Sep 30, 2014 at 01:23:54PM +0200, David Müller (ELSOFT AG) wrote:
fix broken SPI access by adding/activating BOARD_EARLY_INIT_F functionality and calling spi_init_f() from there.
Signed-off-by: David Müller d.mueller@elsoft.ch
Applied to u-boot/master, thanks!
participants (4)
-
David Müller
-
David Müller (ELSOFT AG)
-
Jagan Teki
-
Tom Rini