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?

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/