[U-Boot-Users] Re: PATCH: bi_iic_fast[] initialization

[U-Boot-Users] Re: PATCH: bi_iic_fast[] initialization

[Wolfgang Denk]

Dear Tolunay,

in message <14824.216.110.51.8.1079378410.squirrel@www.orkun.us> you wrote:
> 
>    * Allow bi_iic_fast[] to be initialized via env. variable "i2cfast"

I think you should  rework  this  patch.  At  the  moment,  you  call
getenv() in board_init_f() - but board_init_f() is still running from
flash,  which  means  that we don't have a full C runtime environment
(uninitialized BSS, no writable data, limited stack space) so the use
of getenv() is not allowd in that function. You  may  use  getenv_r()
instead.


Please send a fix.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
The most difficult thing in the world is to know how to  do  a  thing
and to watch someone else doing it wrong, without commenting.
                                                        -- T.H. White

[U-Boot-Users] Re: PATCH: bi_iic_fast[] initialization

[Tolunay Orkun]

Wolfgang,

> Dear Tolunay,
>
>>    * Allow bi_iic_fast[] to be initialized via env. variable "i2cfast"
>
> I think you should  rework  this  patch.  At  the  moment,  you  call
> getenv() in board_init_f() - but board_init_f() is still running from
> flash,  which  means  that we don't have a full C runtime environment
> (uninitialized BSS, no writable data, limited stack space) so the use
> of getenv() is not allowd in that function. You  may  use  getenv_r()
> instead.

I can convert it to use getenv_r() or I think it would be better to move
that section dealing with initialization of bi_iic_fast[] flags to
board_init_r(). bi_iic_fast[] is not used by u-boot itself. It is passed
to Linux.

I believe, there is no reason why it should be done by board_init_f().

Personally, the most appropriate place to initialize such things would be
right before u-boot boots the OS. The immediate changes to the environment
would also be captured without forcing an environment save and reset as
well.

What do you think? Which fix should I attempt:

1) Fix in board_init_f() using getenv_r()
2) Fix by moving to board_init_r()
3) Create a new function (say board_init_preboot) and move that section
into it. Hook board_init_preboot() to the existing boot functions (boot,
bootm etc. somehow)

> Please send a fix.

I will as soon as I get feedback from you.

Regards,
Tolunay

[U-Boot-Users] Re: PATCH: bi_iic_fast[] initialization

[Wolfgang Denk]

In message <30399.216.110.51.8.1081349928.squirrel@www.orkun.us> you wrote:
> 
> > I think you should  rework  this  patch.  At  the  moment,  you  call
> > getenv() in board_init_f() - but board_init_f() is still running from
> > flash,  which  means  that we don't have a full C runtime environment
> > (uninitialized BSS, no writable data, limited stack space) so the use
> > of getenv() is not allowd in that function. You  may  use  getenv_r()
> > instead.
> 
> I can convert it to use getenv_r() or I think it would be better to move
> that section dealing with initialization of bi_iic_fast[] flags to
> board_init_r(). bi_iic_fast[] is not used by u-boot itself. It is passed
> to Linux.

Then move it to board_init_r(), please.

> Personally, the most appropriate place to initialize such things would be
> right before u-boot boots the OS. The immediate changes to the environment
> would also be captured without forcing an environment save and reset as
> well.

Be careful, though. Not everybody uses the same path to boot a kernel.

> 1) Fix in board_init_f() using getenv_r()
> 2) Fix by moving to board_init_r()

Go for 2.

> 3) Create a new function (say board_init_preboot) and move that section
> into it. Hook board_init_preboot() to the existing boot functions (boot,
> bootm etc. somehow)

No, please not yet another xxx_init_pre_before_early_reallybefore_preinit
function.


Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
He had quite a powerful intellect, but it  was  as  powerful  like  a
locomotive,  and  ran on rails and was therefore almost impossible to
steer.                          - Terry Pratchett, _Lords and Ladies_

[U-Boot-Users] PATCH: bi_iic_fast[] initialization relocated to board_init_r()

[Tolunay Orkun]

>> > I think you should  rework  this  patch.  At  the  moment,  you  call
>> > getenv() in board_init_f() - but board_init_f() is still running from
>> > flash,  which  means  that we don't have a full C runtime environment
>> > (uninitialized BSS, no writable data, limited stack space) so the use
>> > of getenv() is not allowd in that function. You  may  use  getenv_r()
>> > instead.
>>
>> I can convert it to use getenv_r() or I think it would be better to move
>> that section dealing with initialization of bi_iic_fast[] flags to
>> board_init_r(). bi_iic_fast[] is not used by u-boot itself. It is passed
>> to Linux.
>
> Then move it to board_init_r(), please.

Attached patch fixes this.

Changelog:

  - Relocate initialization of bi_iic_fast[]
    from board_init_f() to board_init_r()

Best regards,
Tolunay Orkun
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bi_iic_fast_reloc.patch.gz
Type: application/x-gzip
Size: 719 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20040407/fb6f3326/attachment.bin 

[U-Boot-Users] PATCH: bi_iic_fast[] initialization relocated to board_init_r()

[Wolfgang Denk]

In message <33993.216.110.51.8.1081356365.squirrel@www.orkun.us> you wrote:
> 
> Changelog:
> 
>   - Relocate initialization of bi_iic_fast[]
>     from board_init_f() to board_init_r()

Thanks, added.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
The universe is all a spin-off of the Big Bang.