* [U-Boot] [PATCH] spl: avoid printf when we might be doing Ymodem with CONFIG_SPL_FIT_SIGNATURE
@ 2019-03-20 19:52 Alex Kiernan
2019-03-21 0:45 ` Marek Vasut
2019-04-22 16:12 ` [U-Boot] " Tom Rini
0 siblings, 2 replies; 6+ messages in thread
From: Alex Kiernan @ 2019-03-20 19:52 UTC (permalink / raw)
To: u-boot
If CONFIG_SPL_FIT_SIGNATURE is enabled and U-Boot is being loaded from UART
over Ymodem then we can't emit messages using printf() without causing
errors like:
Sending: u-boot-dtb.img
Ymodem sectors/kbytes sent: 3009/376kRetry 0: Got 23 for sector ACK
Retry 0: NAK on sector
Retry 0: Got 68 for sector ACK
Retry 0: NAK on sector
Ymodem sectors/kbytes sent: 3273/409kRetry 0: Got 23 for sector ACK
Retry 0: NAK on sector
Retry 0: Got 68 for sector ACK
Retry 0: NAK on sector
Use debug() rather than printf() to avoid sending messages on the serial
port.
Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
---
common/spl/spl_fit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index db436268cbcd..08faf2c1b058 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -240,12 +240,12 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
}
#ifdef CONFIG_SPL_FIT_SIGNATURE
- printf("## Checking hash(es) for Image %s ... ",
- fit_get_name(fit, node, NULL));
+ debug("## Checking hash(es) for Image %s ... ",
+ fit_get_name(fit, node, NULL));
if (!fit_image_verify_with_data(fit, node,
src, length))
return -EPERM;
- puts("OK\n");
+ debug("OK\n");
#endif
#ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] spl: avoid printf when we might be doing Ymodem with CONFIG_SPL_FIT_SIGNATURE
2019-03-20 19:52 [U-Boot] [PATCH] spl: avoid printf when we might be doing Ymodem with CONFIG_SPL_FIT_SIGNATURE Alex Kiernan
@ 2019-03-21 0:45 ` Marek Vasut
2019-03-21 8:57 ` Alex Kiernan
2019-04-22 16:12 ` [U-Boot] " Tom Rini
1 sibling, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2019-03-21 0:45 UTC (permalink / raw)
To: u-boot
On 3/20/19 8:52 PM, Alex Kiernan wrote:
> If CONFIG_SPL_FIT_SIGNATURE is enabled and U-Boot is being loaded from UART
> over Ymodem then we can't emit messages using printf() without causing
> errors like:
>
> Sending: u-boot-dtb.img
> Ymodem sectors/kbytes sent: 3009/376kRetry 0: Got 23 for sector ACK
> Retry 0: NAK on sector
> Retry 0: Got 68 for sector ACK
> Retry 0: NAK on sector
> Ymodem sectors/kbytes sent: 3273/409kRetry 0: Got 23 for sector ACK
> Retry 0: NAK on sector
> Retry 0: Got 68 for sector ACK
> Retry 0: NAK on sector
>
> Use debug() rather than printf() to avoid sending messages on the serial
> port.
>
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> ---
>
> common/spl/spl_fit.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index db436268cbcd..08faf2c1b058 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -240,12 +240,12 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
> }
>
> #ifdef CONFIG_SPL_FIT_SIGNATURE
> - printf("## Checking hash(es) for Image %s ... ",
> - fit_get_name(fit, node, NULL));
> + debug("## Checking hash(es) for Image %s ... ",
> + fit_get_name(fit, node, NULL));
> if (!fit_image_verify_with_data(fit, node,
> src, length))
> return -EPERM;
> - puts("OK\n");
> + debug("OK\n");
> #endif
Checking printf("Signature failed") to debug() is probably not what you
want to do. It'd be hiding some pretty important output.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH] spl: avoid printf when we might be doing Ymodem with CONFIG_SPL_FIT_SIGNATURE
2019-03-21 0:45 ` Marek Vasut
@ 2019-03-21 8:57 ` Alex Kiernan
0 siblings, 0 replies; 6+ messages in thread
From: Alex Kiernan @ 2019-03-21 8:57 UTC (permalink / raw)
To: u-boot
On Thu, Mar 21, 2019 at 2:58 AM Marek Vasut <marex@denx.de> wrote:
>
> On 3/20/19 8:52 PM, Alex Kiernan wrote:
> > If CONFIG_SPL_FIT_SIGNATURE is enabled and U-Boot is being loaded from UART
> > over Ymodem then we can't emit messages using printf() without causing
> > errors like:
> >
> > Sending: u-boot-dtb.img
> > Ymodem sectors/kbytes sent: 3009/376kRetry 0: Got 23 for sector ACK
> > Retry 0: NAK on sector
> > Retry 0: Got 68 for sector ACK
> > Retry 0: NAK on sector
> > Ymodem sectors/kbytes sent: 3273/409kRetry 0: Got 23 for sector ACK
> > Retry 0: NAK on sector
> > Retry 0: Got 68 for sector ACK
> > Retry 0: NAK on sector
> >
> > Use debug() rather than printf() to avoid sending messages on the serial
> > port.
> >
> > Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> > ---
> >
> > common/spl/spl_fit.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> > index db436268cbcd..08faf2c1b058 100644
> > --- a/common/spl/spl_fit.c
> > +++ b/common/spl/spl_fit.c
> > @@ -240,12 +240,12 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
> > }
> >
> > #ifdef CONFIG_SPL_FIT_SIGNATURE
> > - printf("## Checking hash(es) for Image %s ... ",
> > - fit_get_name(fit, node, NULL));
> > + debug("## Checking hash(es) for Image %s ... ",
> > + fit_get_name(fit, node, NULL));
> > if (!fit_image_verify_with_data(fit, node,
> > src, length))
> > return -EPERM;
> > - puts("OK\n");
> > + debug("OK\n");
> > #endif
>
> Checking printf("Signature failed") to debug() is probably not what you
> want to do. It'd be hiding some pretty important output.
>
I'm not sure what message you do get in that case (since so far I've
failed to get keys into the SPL DTB in a way that seem to actually get
checked), but I take your point.
I can't see any mechanism for either buffering or discarding console
message when Ymodem is active, or am I just missing it, or anything
else that would address this problem?
--
Alex Kiernan
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] spl: avoid printf when we might be doing Ymodem with CONFIG_SPL_FIT_SIGNATURE
2019-03-20 19:52 [U-Boot] [PATCH] spl: avoid printf when we might be doing Ymodem with CONFIG_SPL_FIT_SIGNATURE Alex Kiernan
2019-03-21 0:45 ` Marek Vasut
@ 2019-04-22 16:12 ` Tom Rini
2019-04-22 16:40 ` Marek Vasut
1 sibling, 1 reply; 6+ messages in thread
From: Tom Rini @ 2019-04-22 16:12 UTC (permalink / raw)
To: u-boot
On Wed, Mar 20, 2019 at 07:52:20PM +0000, Alex Kiernan wrote:
> If CONFIG_SPL_FIT_SIGNATURE is enabled and U-Boot is being loaded from UART
> over Ymodem then we can't emit messages using printf() without causing
> errors like:
>
> Sending: u-boot-dtb.img
> Ymodem sectors/kbytes sent: 3009/376kRetry 0: Got 23 for sector ACK
> Retry 0: NAK on sector
> Retry 0: Got 68 for sector ACK
> Retry 0: NAK on sector
> Ymodem sectors/kbytes sent: 3273/409kRetry 0: Got 23 for sector ACK
> Retry 0: NAK on sector
> Retry 0: Got 68 for sector ACK
> Retry 0: NAK on sector
>
> Use debug() rather than printf() to avoid sending messages on the serial
> port.
>
> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> ---
>
> common/spl/spl_fit.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index db436268cbcd..08faf2c1b058 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -240,12 +240,12 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
> }
>
> #ifdef CONFIG_SPL_FIT_SIGNATURE
> - printf("## Checking hash(es) for Image %s ... ",
> - fit_get_name(fit, node, NULL));
> + debug("## Checking hash(es) for Image %s ... ",
> + fit_get_name(fit, node, NULL));
> if (!fit_image_verify_with_data(fit, node,
> src, length))
> return -EPERM;
> - puts("OK\n");
> + debug("OK\n");
> #endif
>
> #ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
I think in this case we want to have that bit of "garbage" happen as the
protocol handles them correctly and if there is a problem it's visible
"on the line" to be seen at least and can be dug at. The other
alternative here would be to re-work the code to only printf anything on
the error case and debug that we're checking at all.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190422/3796151a/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] spl: avoid printf when we might be doing Ymodem with CONFIG_SPL_FIT_SIGNATURE
2019-04-22 16:12 ` [U-Boot] " Tom Rini
@ 2019-04-22 16:40 ` Marek Vasut
2019-04-22 17:02 ` Tom Rini
0 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2019-04-22 16:40 UTC (permalink / raw)
To: u-boot
On 4/22/19 6:12 PM, Tom Rini wrote:
> On Wed, Mar 20, 2019 at 07:52:20PM +0000, Alex Kiernan wrote:
>> If CONFIG_SPL_FIT_SIGNATURE is enabled and U-Boot is being loaded from UART
>> over Ymodem then we can't emit messages using printf() without causing
>> errors like:
>>
>> Sending: u-boot-dtb.img
>> Ymodem sectors/kbytes sent: 3009/376kRetry 0: Got 23 for sector ACK
>> Retry 0: NAK on sector
>> Retry 0: Got 68 for sector ACK
>> Retry 0: NAK on sector
>> Ymodem sectors/kbytes sent: 3273/409kRetry 0: Got 23 for sector ACK
>> Retry 0: NAK on sector
>> Retry 0: Got 68 for sector ACK
>> Retry 0: NAK on sector
>>
>> Use debug() rather than printf() to avoid sending messages on the serial
>> port.
>>
>> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
>> ---
>>
>> common/spl/spl_fit.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index db436268cbcd..08faf2c1b058 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -240,12 +240,12 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
>> }
>>
>> #ifdef CONFIG_SPL_FIT_SIGNATURE
>> - printf("## Checking hash(es) for Image %s ... ",
>> - fit_get_name(fit, node, NULL));
>> + debug("## Checking hash(es) for Image %s ... ",
>> + fit_get_name(fit, node, NULL));
>> if (!fit_image_verify_with_data(fit, node,
>> src, length))
>> return -EPERM;
>> - puts("OK\n");
>> + debug("OK\n");
>> #endif
>>
>> #ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
>
> I think in this case we want to have that bit of "garbage" happen as the
> protocol handles them correctly and if there is a problem it's visible
> "on the line" to be seen at least and can be dug at. The other
> alternative here would be to re-work the code to only printf anything on
> the error case and debug that we're checking at all.
Can we do something similar to this ?
https://patchwork.ozlabs.org/patch/1055047/
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] spl: avoid printf when we might be doing Ymodem with CONFIG_SPL_FIT_SIGNATURE
2019-04-22 16:40 ` Marek Vasut
@ 2019-04-22 17:02 ` Tom Rini
0 siblings, 0 replies; 6+ messages in thread
From: Tom Rini @ 2019-04-22 17:02 UTC (permalink / raw)
To: u-boot
On Mon, Apr 22, 2019 at 06:40:33PM +0200, Marek Vasut wrote:
> On 4/22/19 6:12 PM, Tom Rini wrote:
> > On Wed, Mar 20, 2019 at 07:52:20PM +0000, Alex Kiernan wrote:
> >> If CONFIG_SPL_FIT_SIGNATURE is enabled and U-Boot is being loaded from UART
> >> over Ymodem then we can't emit messages using printf() without causing
> >> errors like:
> >>
> >> Sending: u-boot-dtb.img
> >> Ymodem sectors/kbytes sent: 3009/376kRetry 0: Got 23 for sector ACK
> >> Retry 0: NAK on sector
> >> Retry 0: Got 68 for sector ACK
> >> Retry 0: NAK on sector
> >> Ymodem sectors/kbytes sent: 3273/409kRetry 0: Got 23 for sector ACK
> >> Retry 0: NAK on sector
> >> Retry 0: Got 68 for sector ACK
> >> Retry 0: NAK on sector
> >>
> >> Use debug() rather than printf() to avoid sending messages on the serial
> >> port.
> >>
> >> Signed-off-by: Alex Kiernan <alex.kiernan@gmail.com>
> >> ---
> >>
> >> common/spl/spl_fit.c | 6 +++---
> >> 1 file changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> >> index db436268cbcd..08faf2c1b058 100644
> >> --- a/common/spl/spl_fit.c
> >> +++ b/common/spl/spl_fit.c
> >> @@ -240,12 +240,12 @@ static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
> >> }
> >>
> >> #ifdef CONFIG_SPL_FIT_SIGNATURE
> >> - printf("## Checking hash(es) for Image %s ... ",
> >> - fit_get_name(fit, node, NULL));
> >> + debug("## Checking hash(es) for Image %s ... ",
> >> + fit_get_name(fit, node, NULL));
> >> if (!fit_image_verify_with_data(fit, node,
> >> src, length))
> >> return -EPERM;
> >> - puts("OK\n");
> >> + debug("OK\n");
> >> #endif
> >>
> >> #ifdef CONFIG_SPL_FIT_IMAGE_POST_PROCESS
> >
> > I think in this case we want to have that bit of "garbage" happen as the
> > protocol handles them correctly and if there is a problem it's visible
> > "on the line" to be seen at least and can be dug at. The other
> > alternative here would be to re-work the code to only printf anything on
> > the error case and debug that we're checking at all.
>
> Can we do something similar to this ?
> https://patchwork.ozlabs.org/patch/1055047/
Another valid approach, yes.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190422/4dd81c73/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-22 17:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-20 19:52 [U-Boot] [PATCH] spl: avoid printf when we might be doing Ymodem with CONFIG_SPL_FIT_SIGNATURE Alex Kiernan
2019-03-21 0:45 ` Marek Vasut
2019-03-21 8:57 ` Alex Kiernan
2019-04-22 16:12 ` [U-Boot] " Tom Rini
2019-04-22 16:40 ` Marek Vasut
2019-04-22 17:02 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.