* [PATCH 0/2] readline_simple: return -1 if getc fails
@ 2017-08-07 22:10 Gaël PORTAY
2017-08-07 22:10 ` [PATCH 1/2] " Gaël PORTAY
2017-08-07 22:10 ` [PATCH 2/2] readline_simple: remove obsolete documentation Gaël PORTAY
0 siblings, 2 replies; 16+ messages in thread
From: Gaël PORTAY @ 2017-08-07 22:10 UTC (permalink / raw)
To: barebox; +Cc: Gaël PORTAY
Dear maintainers,
I came accross this issue when I was trying to add support for a new MIPS
board.
The getc() function from console_simple.c returns -EINVAL when no console is
registered.
It causes readline_simple to print a non-printable character (-EINVAL in this
case). Furthermore, it causes an infinite loop: readline never returns.
barebox:/ ���(...)���
With this patch, the error is no more explicit but the readline function stops
to print a dummy character and returns to shell.
barebox:/ <INTERRUPT>
barebox:/ <INTERRUPT>
barebox:/ <INTERRUPT>
(...)
barebox:/ <INTERRUPT>
barebox:/ <INTERRUPT>
barebox:/ <INTERRUPT>
The second patch is a simple cleanup of the in-file documentation.
Regards,
Gaël PORTAY (2):
readline_simple: return -1 if getc fails
readline_simple: remove obsolete documentation
lib/readline_simple.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
--
2.13.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/2] readline_simple: return -1 if getc fails
2017-08-07 22:10 [PATCH 0/2] readline_simple: return -1 if getc fails Gaël PORTAY
@ 2017-08-07 22:10 ` Gaël PORTAY
2017-08-08 7:51 ` Lucas Stach
2017-08-07 22:10 ` [PATCH 2/2] readline_simple: remove obsolete documentation Gaël PORTAY
1 sibling, 1 reply; 16+ messages in thread
From: Gaël PORTAY @ 2017-08-07 22:10 UTC (permalink / raw)
To: barebox; +Cc: Gaël PORTAY
The getc function may return an errno code if an error happens.
This patch prevents readline from printing a non printable character and
from looping to infinity and beyong.
Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
---
lib/readline_simple.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/readline_simple.c b/lib/readline_simple.c
index c4d3d240e..1283c9602 100644
--- a/lib/readline_simple.c
+++ b/lib/readline_simple.c
@@ -58,6 +58,8 @@ int readline (const char *prompt, char *line, int len)
for (;;) {
c = getchar();
+ if (c < 0)
+ return (-1);
/*
* Special character handling
--
2.13.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/2] readline_simple: remove obsolete documentation
2017-08-07 22:10 [PATCH 0/2] readline_simple: return -1 if getc fails Gaël PORTAY
2017-08-07 22:10 ` [PATCH 1/2] " Gaël PORTAY
@ 2017-08-07 22:10 ` Gaël PORTAY
1 sibling, 0 replies; 16+ messages in thread
From: Gaël PORTAY @ 2017-08-07 22:10 UTC (permalink / raw)
To: barebox; +Cc: Gaël PORTAY
The readline function does not return timed-out anymore since the
commit:
be5d13bcb readline_simple: drop unusable u-boot legacy stuff
Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
---
lib/readline_simple.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/lib/readline_simple.c b/lib/readline_simple.c
index 1283c9602..80e075bc5 100644
--- a/lib/readline_simple.c
+++ b/lib/readline_simple.c
@@ -35,11 +35,8 @@ static char * delete_char (char *buffer, char *p, int *colp, int *np, int plen)
/*
* Prompt for input and read a line.
- * If CONFIG_BOOT_RETRY_TIME is defined and retry_time >= 0,
- * time out when time goes past endtime (timebase time in ticks).
* Return: number of read characters
* -1 if break
- * -2 if timed out
*/
int readline (const char *prompt, char *line, int len)
{
--
2.13.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] readline_simple: return -1 if getc fails
2017-08-07 22:10 ` [PATCH 1/2] " Gaël PORTAY
@ 2017-08-08 7:51 ` Lucas Stach
2017-08-08 10:07 ` Ian Abbott
2017-08-08 15:20 ` Gaël PORTAY
0 siblings, 2 replies; 16+ messages in thread
From: Lucas Stach @ 2017-08-08 7:51 UTC (permalink / raw)
To: Gaël PORTAY; +Cc: barebox
Am Montag, den 07.08.2017, 18:10 -0400 schrieb Gaël PORTAY:
> The getc function may return an errno code if an error happens.
>
> This patch prevents readline from printing a non printable character and
> from looping to infinity and beyong.
>
> Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
> ---
> lib/readline_simple.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/lib/readline_simple.c b/lib/readline_simple.c
> index c4d3d240e..1283c9602 100644
> --- a/lib/readline_simple.c
> +++ b/lib/readline_simple.c
> @@ -58,6 +58,8 @@ int readline (const char *prompt, char *line, int len)
>
> for (;;) {
> c = getchar();
> + if (c < 0)
> + return (-1);
I don't like made up error codes. Is there any reason why we couldn't
just pass through the negative error code from getchar?
Regards,
Lucas
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] readline_simple: return -1 if getc fails
2017-08-08 7:51 ` Lucas Stach
@ 2017-08-08 10:07 ` Ian Abbott
2017-08-08 15:21 ` Gaël PORTAY
2017-08-08 15:20 ` Gaël PORTAY
1 sibling, 1 reply; 16+ messages in thread
From: Ian Abbott @ 2017-08-08 10:07 UTC (permalink / raw)
To: Lucas Stach, Gaël PORTAY; +Cc: barebox
On 08/08/17 08:51, Lucas Stach wrote:
> Am Montag, den 07.08.2017, 18:10 -0400 schrieb Gaël PORTAY:
>> The getc function may return an errno code if an error happens.
>>
>> This patch prevents readline from printing a non printable character and
>> from looping to infinity and beyong.
>>
>> Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
>> ---
>> lib/readline_simple.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/lib/readline_simple.c b/lib/readline_simple.c
>> index c4d3d240e..1283c9602 100644
>> --- a/lib/readline_simple.c
>> +++ b/lib/readline_simple.c
>> @@ -58,6 +58,8 @@ int readline (const char *prompt, char *line, int len)
>>
>> for (;;) {
>> c = getchar();
>> + if (c < 0)
>> + return (-1);
>
> I don't like made up error codes. Is there any reason why we couldn't
> just pass through the negative error code from getchar?
And also change the type of the 'c' variable, as it is currently a plain
'char'?
> Regards,
> Lucas
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@mev.co.uk> )=-
-=( Web: http://www.mev.co.uk/ )=-
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] readline_simple: return -1 if getc fails
2017-08-08 7:51 ` Lucas Stach
2017-08-08 10:07 ` Ian Abbott
@ 2017-08-08 15:20 ` Gaël PORTAY
2017-08-08 15:36 ` Lucas Stach
1 sibling, 1 reply; 16+ messages in thread
From: Gaël PORTAY @ 2017-08-08 15:20 UTC (permalink / raw)
To: Lucas Stach; +Cc: barebox
Hi Lucas,
On Tue, Aug 08, 2017 at 09:51:54AM +0200, Lucas Stach wrote:
> Am Montag, den 07.08.2017, 18:10 -0400 schrieb Gaël PORTAY:
> > The getc function may return an errno code if an error happens.
> >
> > This patch prevents readline from printing a non printable character and
> > from looping to infinity and beyong.
> >
> > Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
> > ---
> > lib/readline_simple.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/readline_simple.c b/lib/readline_simple.c
> > index c4d3d240e..1283c9602 100644
> > --- a/lib/readline_simple.c
> > +++ b/lib/readline_simple.c
> > @@ -58,6 +58,8 @@ int readline (const char *prompt, char *line, int len)
> >
> > for (;;) {
> > c = getchar();
> > + if (c < 0)
> > + return (-1);
>
> I don't like made up error codes. Is there any reason why we couldn't
> just pass through the negative error code from getchar?
>
The thing here is that getchar() may return an error, and that error is not
tested. This causes readline to print the character 0xea (-EINVAL) which is not
printable.
Maybe another solutions would be to print the errno string and continue; or to
return the number of characters already read.
Regards,
Gaël
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] readline_simple: return -1 if getc fails
2017-08-08 10:07 ` Ian Abbott
@ 2017-08-08 15:21 ` Gaël PORTAY
0 siblings, 0 replies; 16+ messages in thread
From: Gaël PORTAY @ 2017-08-08 15:21 UTC (permalink / raw)
To: Ian Abbott; +Cc: barebox
Hi Ian,
On Tue, Aug 08, 2017 at 11:07:54AM +0100, Ian Abbott wrote:
> On 08/08/17 08:51, Lucas Stach wrote:
> > Am Montag, den 07.08.2017, 18:10 -0400 schrieb Gaël PORTAY:
> > > The getc function may return an errno code if an error happens.
> > >
> > > This patch prevents readline from printing a non printable character and
> > > from looping to infinity and beyong.
> > >
> > > Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
> > > ---
> > > lib/readline_simple.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/lib/readline_simple.c b/lib/readline_simple.c
> > > index c4d3d240e..1283c9602 100644
> > > --- a/lib/readline_simple.c
> > > +++ b/lib/readline_simple.c
> > > @@ -58,6 +58,8 @@ int readline (const char *prompt, char *line, int len)
> > > for (;;) {
> > > c = getchar();
> > > + if (c < 0)
> > > + return (-1);
> >
> > I don't like made up error codes. Is there any reason why we couldn't
> > just pass through the negative error code from getchar?
>
> And also change the type of the 'c' variable, as it is currently a plain
> 'char'?
>
Indeed :)
Regards,
Gaël
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] readline_simple: return -1 if getc fails
2017-08-08 15:20 ` Gaël PORTAY
@ 2017-08-08 15:36 ` Lucas Stach
2017-08-08 16:05 ` Ian Abbott
2017-08-08 16:14 ` Gaël PORTAY
0 siblings, 2 replies; 16+ messages in thread
From: Lucas Stach @ 2017-08-08 15:36 UTC (permalink / raw)
To: Gaël PORTAY; +Cc: barebox
Am Dienstag, den 08.08.2017, 11:20 -0400 schrieb Gaël PORTAY:
> Hi Lucas,
>
> On Tue, Aug 08, 2017 at 09:51:54AM +0200, Lucas Stach wrote:
> > Am Montag, den 07.08.2017, 18:10 -0400 schrieb Gaël PORTAY:
> > > The getc function may return an errno code if an error happens.
> > >
> > > This patch prevents readline from printing a non printable character and
> > > from looping to infinity and beyong.
> > >
> > > Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
> > > ---
> > > lib/readline_simple.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/lib/readline_simple.c b/lib/readline_simple.c
> > > index c4d3d240e..1283c9602 100644
> > > --- a/lib/readline_simple.c
> > > +++ b/lib/readline_simple.c
> > > @@ -58,6 +58,8 @@ int readline (const char *prompt, char *line, int len)
> > >
> > > for (;;) {
> > > c = getchar();
> > > + if (c < 0)
> > > + return (-1);
> >
> > I don't like made up error codes. Is there any reason why we couldn't
> > just pass through the negative error code from getchar?
> >
>
> The thing here is that getchar() may return an error, and that error is not
> tested. This causes readline to print the character 0xea (-EINVAL) which is not
> printable.
So why wouldn't the following fix the issue?
signed char c;
if (c < 0)
return c;
Regards,
Lucas
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] readline_simple: return -1 if getc fails
2017-08-08 15:36 ` Lucas Stach
@ 2017-08-08 16:05 ` Ian Abbott
2017-08-08 16:49 ` Gaël PORTAY
2017-08-08 16:14 ` Gaël PORTAY
1 sibling, 1 reply; 16+ messages in thread
From: Ian Abbott @ 2017-08-08 16:05 UTC (permalink / raw)
To: Lucas Stach, Gaël PORTAY; +Cc: barebox
On 08/08/17 16:36, Lucas Stach wrote:
> Am Dienstag, den 08.08.2017, 11:20 -0400 schrieb Gaël PORTAY:
>> Hi Lucas,
>>
>> On Tue, Aug 08, 2017 at 09:51:54AM +0200, Lucas Stach wrote:
>>> Am Montag, den 07.08.2017, 18:10 -0400 schrieb Gaël PORTAY:
>>>> The getc function may return an errno code if an error happens.
>>>>
>>>> This patch prevents readline from printing a non printable character and
>>>> from looping to infinity and beyong.
>>>>
>>>> Signed-off-by: Gaël PORTAY <gael.portay@savoirfairelinux.com>
>>>> ---
>>>> lib/readline_simple.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/lib/readline_simple.c b/lib/readline_simple.c
>>>> index c4d3d240e..1283c9602 100644
>>>> --- a/lib/readline_simple.c
>>>> +++ b/lib/readline_simple.c
>>>> @@ -58,6 +58,8 @@ int readline (const char *prompt, char *line, int len)
>>>>
>>>> for (;;) {
>>>> c = getchar();
>>>> + if (c < 0)
>>>> + return (-1);
>>>
>>> I don't like made up error codes. Is there any reason why we couldn't
>>> just pass through the negative error code from getchar?
>>>
>>
>> The thing here is that getchar() may return an error, and that error is not
>> tested. This causes readline to print the character 0xea (-EINVAL) which is not
>> printable.
>
> So why wouldn't the following fix the issue?
>
> signed char c;
`int` would be better to allow non-ASCII characters.
>
> if (c < 0)
> return c;
There are places where the return value is checked for `-1` for example
in get_user_input() ("common/hush.c"), and in run_shell()
("common/parser.c").
I think Gaël's patch is reasonable, although perhaps it should also set
`line[0] = '\0';` before returning.
Off topic: there is another oddity in the the "simple" version of
readline(). It ignores the `len` parameter and uses `CONFIG_CBSIZE` instead.
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@mev.co.uk> )=-
-=( Web: http://www.mev.co.uk/ )=-
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] readline_simple: return -1 if getc fails
2017-08-08 15:36 ` Lucas Stach
2017-08-08 16:05 ` Ian Abbott
@ 2017-08-08 16:14 ` Gaël PORTAY
1 sibling, 0 replies; 16+ messages in thread
From: Gaël PORTAY @ 2017-08-08 16:14 UTC (permalink / raw)
To: Lucas Stach; +Cc: barebox
On Tue, Aug 08, 2017 at 05:36:14PM +0200, Lucas Stach wrote:
> Am Dienstag, den 08.08.2017, 11:20 -0400 schrieb Gaël PORTAY:
> > Hi Lucas,
> >
> > On Tue, Aug 08, 2017 at 09:51:54AM +0200, Lucas Stach wrote:
> > > > diff --git a/lib/readline_simple.c b/lib/readline_simple.c
> > > > index c4d3d240e..1283c9602 100644
> > > > --- a/lib/readline_simple.c
> > > > +++ b/lib/readline_simple.c
> > > > @@ -58,6 +58,8 @@ int readline (const char *prompt, char *line, int len)
> > > >
> > > > for (;;) {
> > > > c = getchar();
> > > > + if (c < 0)
> > > > + return (-1);
> > >
> > > I don't like made up error codes. Is there any reason why we couldn't
> > > just pass through the negative error code from getchar?
> > >
> >
> > The thing here is that getchar() may return an error, and that error is not
> > tested. This causes readline to print the character 0xea (-EINVAL) which is not
> > printable.
>
> So why wouldn't the following fix the issue?
>
> signed char c;
>
> if (c < 0)
> return c;
>
Okay. I do prefer your solution.
I returned -1 mainly because the function comment says it returns -1 when it
breaks; and because parser.c and hush.c test readline function against -1 and
not against a negative value.
Also readline returns -1 if character is 0x03. Maybe it should return -EINTR;
-1 is EPERM: Operation not permitted.
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] readline_simple: return -1 if getc fails
2017-08-08 16:05 ` Ian Abbott
@ 2017-08-08 16:49 ` Gaël PORTAY
0 siblings, 0 replies; 16+ messages in thread
From: Gaël PORTAY @ 2017-08-08 16:49 UTC (permalink / raw)
To: Ian Abbott; +Cc: barebox
On Tue, Aug 08, 2017 at 05:05:35PM +0100, Ian Abbott wrote:
> On 08/08/17 16:36, Lucas Stach wrote:
> > Am Dienstag, den 08.08.2017, 11:20 -0400 schrieb Gaël PORTAY:
> > > Hi Lucas,
> > >
> > > On Tue, Aug 08, 2017 at 09:51:54AM +0200, Lucas Stach wrote:
> > > > (...)
> > > > I don't like made up error codes. Is there any reason why we couldn't
> > > > just pass through the negative error code from getchar?
> > > >
> > >
> > > The thing here is that getchar() may return an error, and that error is not
> > > tested. This causes readline to print the character 0xea (-EINVAL) which is not
> > > printable.
> >
> > So why wouldn't the following fix the issue?
> >
> > signed char c;
>
> `int` would be better to allow non-ASCII characters.
>
> >
> > if (c < 0)
> > return c;
>
> There are places where the return value is checked for `-1` for example in
> get_user_input() ("common/hush.c"), and in run_shell() ("common/parser.c").
>
> I think Gaël's patch is reasonable, although perhaps it should also set
> `line[0] = '\0';` before returning.
>
Indeed, or `line[n] = '\0';` to preserve characters already entered.
BTW, it already performed by get_user_input in hush.c (n is reset to 0).
console_buffer[n] = '\n';
console_buffer[n + 1]= '\0';
> Off topic: there is another oddity in the the "simple" version of
> readline(). It ignores the `len` parameter and uses `CONFIG_CBSIZE` instead.
>
I made a patch for this; but I have not tested yet all cases.
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] readline_simple: return -1 if getc fails
2017-08-09 18:58 ` Gaël PORTAY
2017-08-09 19:41 ` Oleksij Rempel
@ 2017-08-10 5:51 ` Antony Pavlov
1 sibling, 0 replies; 16+ messages in thread
From: Antony Pavlov @ 2017-08-10 5:51 UTC (permalink / raw)
To: Gaël PORTAY; +Cc: barebox, Oleksij Rempel
On Wed, 9 Aug 2017 14:58:53 -0400
Gaël PORTAY <gael.portay@savoirfairelinux.com> wrote:
> Hi,
>
> On Wed, Aug 09, 2017 at 08:05:57AM +0200, Oleksij Rempel wrote:
> > Am 09.08.2017 um 06:13 schrieb Gaël PORTAY:
> > > Dear maintainers,
> > >
> > > I came accross this issue when I was trying to add support for a new MIPS
> > > board.
> >
> > Just out of curiosity, what MIPS board are you working on? If it is not
> > a secret :)
> >
>
> I am working on VoCore2[1]; it is a board based on a mediatek mt7628 CPU.
>
> I am doing this to learn myself how works barebox
Please see this branch for mt7628:
https://github.com/frantony/barebox/tree/20160815.mediatek
>
> Offtopic: For now it is booting on a really really old version of u-boot. I am
> able de boot barebox from u-boot. I had to hack the mips_disable_interrupts to
> remove the reset of flag ERL in CP0; but I don't know why yet. This flag is
> supposed to be set when CPU reboots; but because I am running barebox from a the
> command go of u-boot, this flag is unset.
>
> Are you able to re-run barebox from barebox on your MIPS board?
>
> .macro mips_disable_interrupts
> .set push
> .set noreorder
> mfc0 k0, CP0_STATUS
> - li k1, ~(ST0_ERL | ST0_IE)
> + li k1, ~ST0_IE
> and k0, k1
> mtc0 k0, CP0_STATUS
> .set pop
> .endm
>
> For now, I have splitted mips_disable_interrupts into two macros. The second
> macro mips_reset_error_level checks if the ERL flag is armed before resetting it
> to 0.
>
> .macro mips_reset_error_level
> .set push
> .set noreorder
> mfc0 k0, CP0_STATUS
> li k1, ST0_ERL
> and k1, k0
> bne k1, zero, 1f
> li k1, ~(ST0_ERL)
> and k0, k1
> mtc0 k0, CP0_STATUS
> 1: .set pop
> .endm
>
> But I don't know if this is the right thing to do.
>
> [1]: https://www.indiegogo.com/projects/vocore2-4-coin-sized-linux-computer-with-wifi#/
>
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
--
Best regards,
Antony Pavlov
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] readline_simple: return -1 if getc fails
2017-08-09 18:58 ` Gaël PORTAY
@ 2017-08-09 19:41 ` Oleksij Rempel
2017-08-10 5:51 ` Antony Pavlov
1 sibling, 0 replies; 16+ messages in thread
From: Oleksij Rempel @ 2017-08-09 19:41 UTC (permalink / raw)
To: Gaël PORTAY; +Cc: barebox
[-- Attachment #1.1.1: Type: text/plain, Size: 2291 bytes --]
Am 09.08.2017 um 20:58 schrieb Gaël PORTAY:
> Hi,
>
> On Wed, Aug 09, 2017 at 08:05:57AM +0200, Oleksij Rempel wrote:
>> Am 09.08.2017 um 06:13 schrieb Gaël PORTAY:
>>> Dear maintainers,
>>>
>>> I came accross this issue when I was trying to add support for a new MIPS
>>> board.
>>
>> Just out of curiosity, what MIPS board are you working on? If it is not
>> a secret :)
>>
>
> I am working on VoCore2[1]; it is a board based on a mediatek mt7628 CPU.
>
> I am doing this to learn myself how works barebox
>
> Offtopic: For now it is booting on a really really old version of u-boot. I am
> able de boot barebox from u-boot. I had to hack the mips_disable_interrupts to
> remove the reset of flag ERL in CP0; but I don't know why yet. This flag is
> supposed to be set when CPU reboots; but because I am running barebox from a the
> command go of u-boot, this flag is unset.
>
> Are you able to re-run barebox from barebox on your MIPS board?
Yes, usually i prefer to make a binaries wich are able to start from
RAM, Jtag and Flash. Currently we test if it is running from spi flash
adn skip most of the low level init:
https://github.com/olerem/barebox/blob/ar9344-2017.08.01.1/arch/mips/boards/tplink-mr3020/include/board/board_pbl_start.h#L29
On this board i decided to use SRAM for low level init testing:
https://github.com/olerem/barebox/blob/ar9344-2017.08.01.1/arch/mips/boards/tplink-wdr4300/include/board/board_pbl_start.h#L38
> .macro mips_disable_interrupts
> .set push
> .set noreorder
> mfc0 k0, CP0_STATUS
> - li k1, ~(ST0_ERL | ST0_IE)
> + li k1, ~ST0_IE
> and k0, k1
> mtc0 k0, CP0_STATUS
> .set pop
> .endm
>
> For now, I have splitted mips_disable_interrupts into two macros. The second
> macro mips_reset_error_level checks if the ERL flag is armed before resetting it
> to 0.
>
> .macro mips_reset_error_level
> .set push
> .set noreorder
> mfc0 k0, CP0_STATUS
> li k1, ST0_ERL
> and k1, k0
> bne k1, zero, 1f
> li k1, ~(ST0_ERL)
> and k0, k1
> mtc0 k0, CP0_STATUS
> 1: .set pop
> .endm
>
> But I don't know if this is the right thing to do.
>
> [1]: https://www.indiegogo.com/projects/vocore2-4-coin-sized-linux-computer-with-wifi#/
>
--
Regards,
Oleksij
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 149 bytes --]
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] readline_simple: return -1 if getc fails
2017-08-09 6:05 ` Oleksij Rempel
@ 2017-08-09 18:58 ` Gaël PORTAY
2017-08-09 19:41 ` Oleksij Rempel
2017-08-10 5:51 ` Antony Pavlov
0 siblings, 2 replies; 16+ messages in thread
From: Gaël PORTAY @ 2017-08-09 18:58 UTC (permalink / raw)
To: Oleksij Rempel; +Cc: barebox
Hi,
On Wed, Aug 09, 2017 at 08:05:57AM +0200, Oleksij Rempel wrote:
> Am 09.08.2017 um 06:13 schrieb Gaël PORTAY:
> > Dear maintainers,
> >
> > I came accross this issue when I was trying to add support for a new MIPS
> > board.
>
> Just out of curiosity, what MIPS board are you working on? If it is not
> a secret :)
>
I am working on VoCore2[1]; it is a board based on a mediatek mt7628 CPU.
I am doing this to learn myself how works barebox
Offtopic: For now it is booting on a really really old version of u-boot. I am
able de boot barebox from u-boot. I had to hack the mips_disable_interrupts to
remove the reset of flag ERL in CP0; but I don't know why yet. This flag is
supposed to be set when CPU reboots; but because I am running barebox from a the
command go of u-boot, this flag is unset.
Are you able to re-run barebox from barebox on your MIPS board?
.macro mips_disable_interrupts
.set push
.set noreorder
mfc0 k0, CP0_STATUS
- li k1, ~(ST0_ERL | ST0_IE)
+ li k1, ~ST0_IE
and k0, k1
mtc0 k0, CP0_STATUS
.set pop
.endm
For now, I have splitted mips_disable_interrupts into two macros. The second
macro mips_reset_error_level checks if the ERL flag is armed before resetting it
to 0.
.macro mips_reset_error_level
.set push
.set noreorder
mfc0 k0, CP0_STATUS
li k1, ST0_ERL
and k1, k0
bne k1, zero, 1f
li k1, ~(ST0_ERL)
and k0, k1
mtc0 k0, CP0_STATUS
1: .set pop
.endm
But I don't know if this is the right thing to do.
[1]: https://www.indiegogo.com/projects/vocore2-4-coin-sized-linux-computer-with-wifi#/
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/2] readline_simple: return -1 if getc fails
2017-08-09 4:13 [PATCH 0/2] readline_simple: return -1 if getc fails Gaël PORTAY
@ 2017-08-09 6:05 ` Oleksij Rempel
2017-08-09 18:58 ` Gaël PORTAY
0 siblings, 1 reply; 16+ messages in thread
From: Oleksij Rempel @ 2017-08-09 6:05 UTC (permalink / raw)
To: barebox
[-- Attachment #1.1.1: Type: text/plain, Size: 276 bytes --]
Am 09.08.2017 um 06:13 schrieb Gaël PORTAY:
> Dear maintainers,
>
> I came accross this issue when I was trying to add support for a new MIPS
> board.
Just out of curiosity, what MIPS board are you working on? If it is not
a secret :)
--
Regards,
Oleksij
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]
[-- Attachment #2: Type: text/plain, Size: 149 bytes --]
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/2] readline_simple: return -1 if getc fails
@ 2017-08-09 4:13 Gaël PORTAY
2017-08-09 6:05 ` Oleksij Rempel
0 siblings, 1 reply; 16+ messages in thread
From: Gaël PORTAY @ 2017-08-09 4:13 UTC (permalink / raw)
To: barebox; +Cc: Gaël PORTAY
Dear maintainers,
I came accross this issue when I was trying to add support for a new MIPS
board.
The getc() function from console_simple.c returns -EINVAL when no console is
registered.
It causes readline_simple to print a non-printable character (-EINVAL in this
case). Furthermore, it causes an infinite loop: readline never returns.
barebox:/ ���(...)���
With this patch, the error is no more explicit but the readline function stops
to print a dummy character and returns to shell.
barebox:/ <INTERRUPT>
barebox:/ <INTERRUPT>
barebox:/ <INTERRUPT>
(...)
barebox:/ <INTERRUPT>
barebox:/ <INTERRUPT>
barebox:/ <INTERRUPT>
The second patch is a simple cleanup of the in-file documentation.
Changes since v1:
- change c variable type from char in int
- set *p to NUL before returning -1 when getchar returns an error
- update function documentation
Regards,
Gaël PORTAY (2):
readline_simple: return -1 if getc fails
readline_simple: remove obsolete documentation
lib/readline_simple.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
--
2.13.2
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-08-10 5:41 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 22:10 [PATCH 0/2] readline_simple: return -1 if getc fails Gaël PORTAY
2017-08-07 22:10 ` [PATCH 1/2] " Gaël PORTAY
2017-08-08 7:51 ` Lucas Stach
2017-08-08 10:07 ` Ian Abbott
2017-08-08 15:21 ` Gaël PORTAY
2017-08-08 15:20 ` Gaël PORTAY
2017-08-08 15:36 ` Lucas Stach
2017-08-08 16:05 ` Ian Abbott
2017-08-08 16:49 ` Gaël PORTAY
2017-08-08 16:14 ` Gaël PORTAY
2017-08-07 22:10 ` [PATCH 2/2] readline_simple: remove obsolete documentation Gaël PORTAY
2017-08-09 4:13 [PATCH 0/2] readline_simple: return -1 if getc fails Gaël PORTAY
2017-08-09 6:05 ` Oleksij Rempel
2017-08-09 18:58 ` Gaël PORTAY
2017-08-09 19:41 ` Oleksij Rempel
2017-08-10 5:51 ` Antony Pavlov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox