[U-Boot] putc

[Simon Glass]

Hi Martin,

On 13 November 2014 03:28, Martin Dorwig <dorwig at tetronik.com> wrote:
> Am Mittwoch, 12. November 2014, 19:32:21 schrieben Sie:
>> Hi Martin,
>>
>> On 30 October 2014 00:45, Martin Dorwig <dorwig at tetronik.com> wrote:
>> > Am Mittwoch, 29. Oktober 2014, 13:03:41 schrieben Sie:
>> >> Hi Martin,
>> >>
>> >> On 28 October 2014 07:23, Martin Dorwig <dorwig at tetronik.com> wrote:
>> >> > Hello,
>> >> > after updating u-boot from git v2014.10
>> >> > i just noticed that my standalone  application outputs garbage using
>> >> > putc.
>> >> > after some investigation i found that the gd->jt[XF_putc] is assigned a
>> >> > dev->putc pointer in "common/console.c function console_setfile"
>> >> >
>> >> > the dev->putc is declared in include/stdio_dev.h as
>> >> > void (*putc)(struct stdio_dev *dev, const char c);
>> >> >
>> >> > int include/exports.h this function is declared as
>> >> > void putc(const char);
>> >> >
>> >> > this was introduced with commit id
>> >> > 709ea543b92489e7729d2d7ddd6c9f451e52158c
>> >>
>> >> That's unfortunate. Is the stdio device pointer available in the
>> >> standalone application?
>> >>
>> >> Perhaps gd->jt should point to a structure of strongly-typed
>> >> functions? Then we would get a build error in this case.
>> >>
>> >> Regards,
>> >> Simon
>> >
>> > What is the motivation to override the jumptable slots ?
>> > Why can't they be untouched like this
>> > gd->jt[XF_getc] = getc;
>> > gd->jt[XF_tstc] = tstc;
>> > gd->jt[XF_putc] = putc;
>> > gd->jt[XF_puts] = puts;
>> > redirection of the console to another device is honored in the
>> > implementation of the functions themselves.
>> > for example the putc function:
>> > it calls pre_console_putc before a console is present
>> > if GD_FLG_DEVINIT is set, it calls fputc(stdout,c) which calls
>> > console_putc(stdout,c) which in turn calls stdio_devices[stdout]-
>> >
>> >>putc(stdio_devices[file], c);
>> >>
>> > else it calls serial_putc(c)
>>
>> I have taken a bit of a look at this.
>>
>> I have not actually touched the jumptable slots. The one you mention
>> was assigned to dev->putc() before and after. The problem is that now
>> dev->putc() has two parameters, the first being 'dev'.
>>
>> dev->putc() is the device's putc function - typically the serial port
>> or a display.
>>
>> The putc() function you mention is the console out, that outputs to
>> all stdout devices.
>>
>> So yes we could change the bahaviour so that it calls putc(). I think
>> this will work, and it probably isn't going to result in a change in
>> behaviour for many boards since they only have a single stdout.
>>
>> Alternatively we could require the stdio device parameter to be passed
>> in. I'm not sure that will work though - how will the external program
>> get at it?
>>
>> In any case, teh void *gd->jt[] is not serving our purpose here - it
>> should be changed to proper function pointer types so that the
>> compiler can check that this sort of thing doesn't happen.
>>
>> Please let me know what you think should be done here.
>>
>> Regards,
>> Simon
>
> The jumptable is initialized once, and i don't see any reason to overwrite it.

That commit (27b207fd) dates from 2003. Are you wanting to revert an
11-year-old commit or do I misunderstand you?

> Adding a parameter to these exported functions will break backward
> compatibility
> of existing standalone apps

Yes that's right, but when they move to a newer U-Boot they will need
to be updated. Isn't that expected with standalone apps? I can adjust
things that are in the mainline tree, but I can't fix up people's
third party apps.

I did make a suggestion about how to avoid adding the extra parameter
(changing the behaviour to call putc() instead of dev->putc()). What
do you think?

Regards,
Simon