| Message ID | 1440769821-24005-2-git-send-email-l.majewski@samsung.com |
|---|---|
| State | Superseded |
| Delegated to: | Pantelis Antoniou |
| Headers | show |
Hi Lukasz, On 28 August 2015 at 07:50, Lukasz Majewski <l.majewski@samsung.com> wrote: > > The timeout error for DW MMC transfer should be visible on the u-boot > console to speed up the process of debugging. > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > Cc: Tom Rini <trini@konsulko.com> > Cc: Simon Glass <sjg@chromium.org> > --- > drivers/mmc/dw_mmc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Can this be reported at the command line level instead? It feels wrong to be printing errors in a driver. Also, why does it return TIMEOUT instead of -ETIMEOUT? Regards, Simon
On Fri, 28 Aug 2015 17:21:51 -0600 Simon Glass <sjg@chromium.org> wrote: > Hi Lukasz, > > On 28 August 2015 at 07:50, Lukasz Majewski <l.majewski@samsung.com> > wrote: > > > > The timeout error for DW MMC transfer should be visible on the > > u-boot console to speed up the process of debugging. > > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > > Cc: Tom Rini <trini@konsulko.com> > > Cc: Simon Glass <sjg@chromium.org> > > --- > > drivers/mmc/dw_mmc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Can this be reported at the command line level instead? With my setup - dfu tests - I didn't receive any error/information about the MMC (SD Card to be precise) timeout. (I would expect MMC subsystem to return -ETIMEOUT and then this error would be propagated to dfu and stop execution of the dfu command). This didn't work and hence should be scrutinized. > It feels wrong > to be printing errors in a driver. If this information would be printed on the console I might have noticed it immediately and save some time for debugging. > Also, why does it return TIMEOUT > instead of -ETIMEOUT? Good question. > > Regards, > Simon Best regards, Lukasz Majewski
Hi Lukasz, On 29 August 2015 at 06:09, Lukasz Majewski <l.majewski@majess.pl> wrote: > On Fri, 28 Aug 2015 17:21:51 -0600 > Simon Glass <sjg@chromium.org> wrote: > >> Hi Lukasz, >> >> On 28 August 2015 at 07:50, Lukasz Majewski <l.majewski@samsung.com> >> wrote: >> > >> > The timeout error for DW MMC transfer should be visible on the >> > u-boot console to speed up the process of debugging. >> > >> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> >> > Cc: Tom Rini <trini@konsulko.com> >> > Cc: Simon Glass <sjg@chromium.org> >> > --- >> > drivers/mmc/dw_mmc.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> >> Can this be reported at the command line level instead? > > With my setup - dfu tests - I didn't receive any error/information > about the MMC (SD Card to be precise) timeout. > > (I would expect MMC subsystem to return -ETIMEOUT and then this error > would be propagated to dfu and stop execution of the dfu command). > > This didn't work and hence should be scrutinized. Agreed. > >> It feels wrong >> to be printing errors in a driver. > > If this information would be printed on the console I might have > noticed it immediately and save some time for debugging. > >> Also, why does it return TIMEOUT >> instead of -ETIMEOUT? > > Good question. It looks likes mmc.h has its own errors (NO_CARD_ERR, etc.). I suspect these could be converted to use standard errors. Anyway your question remains. A driver error should be reported by the caller. Regards, Simon
Hi Simon, > Hi Lukasz, > > On 29 August 2015 at 06:09, Lukasz Majewski <l.majewski@majess.pl> > wrote: > > On Fri, 28 Aug 2015 17:21:51 -0600 > > Simon Glass <sjg@chromium.org> wrote: > > > >> Hi Lukasz, > >> > >> On 28 August 2015 at 07:50, Lukasz Majewski > >> <l.majewski@samsung.com> wrote: > >> > > >> > The timeout error for DW MMC transfer should be visible on the > >> > u-boot console to speed up the process of debugging. > >> > > >> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > >> > Cc: Tom Rini <trini@konsulko.com> > >> > Cc: Simon Glass <sjg@chromium.org> > >> > --- > >> > drivers/mmc/dw_mmc.c | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> Can this be reported at the command line level instead? > > > > With my setup - dfu tests - I didn't receive any error/information > > about the MMC (SD Card to be precise) timeout. > > > > (I would expect MMC subsystem to return -ETIMEOUT and then this > > error would be propagated to dfu and stop execution of the dfu > > command). > > > > This didn't work and hence should be scrutinized. > > Agreed. > > > > >> It feels wrong > >> to be printing errors in a driver. > > > > If this information would be printed on the console I might have > > noticed it immediately and save some time for debugging. > > > >> Also, why does it return TIMEOUT > >> instead of -ETIMEOUT? > > > > Good question. > > It looks likes mmc.h has its own errors (NO_CARD_ERR, etc.). I suspect > these could be converted to use standard errors. > > Anyway your question remains. A driver error should be reported by > the caller. Please drop this patch. I've posted other one (as a reply tp this one): "[PATCH] FIX: fat: Provide correct return code from disk_{read|write}" to fix this problem. > > Regards, > Simon >
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 21a92d2..98f5cb7 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -231,7 +231,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, /* Check for timeout. */ if (get_timer(start) > timeout) { - debug("%s: Timeout waiting for data!\n", + printf("%s: Timeout waiting for data!\n", __func__); ret = TIMEOUT; break;
The timeout error for DW MMC transfer should be visible on the u-boot console to speed up the process of debugging. Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> Cc: Tom Rini <trini@konsulko.com> Cc: Simon Glass <sjg@chromium.org> --- drivers/mmc/dw_mmc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)