Re: [U-Boot] [PATCH] dw_mmc: cleanups
Hi,
On Thu, 2014-09-11 at 11:06 +0900, Jaehoon Chung wrote:
Hi, Pavel.
It looks good to me.
If you're ok, can i suggest one thing?
On 09/05/2014 07:49 PM, Pavel Machek wrote:
dw_mmc driver was responding to errors with debug(). Change that to
prinf so that any errors are immediately obvious. Also adjust english
in comments.
Signed-off-by: Pavel Machek pa...@denx.de
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 0df30bc..4c16e7f 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -260,7 +262,7 @@ static int dwmci_setup_bus(struct dwmci_host *host, u32
freq)
do {
status = dwmci_readl(host, DWMCI_CMD);
if (timeout-- 0) {
- printf(TIMEOUT error!!\n);
+ printf(dwmci_setup_bus: timeout!\n);
return -ETIMEDOUT;
}
} while (status DWMCI_CMD_START);
@@ -275,7 +277,7 @@ static int dwmci_setup_bus(struct dwmci_host *host, u32
freq)
do {
status = dwmci_readl(host, DWMCI_CMD);
if (timeout-- 0) {
- printf(TIMEOUT error!!\n);
+ printf(dwmci_setup_bus: timeout!\n);
According to your the main purpose, this patch is goal that noticed
where/what error is occurred.
Then i think good that add the __LINE__.
Because there are same message like dwmci_setup_bus: tiemout!\n at some
place.
Then i think we don't need to discuss about this patch with Marek. :)
This patch looks good except need to distinguish the error location as
pointed by Jaehoon.
Thanks
Chin Liang
Best Regards,
Jaehoon Chung
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dw_mmc: cleanups
Hi, Pavel.
It looks good to me.
If you're ok, can i suggest one thing?
On 09/05/2014 07:49 PM, Pavel Machek wrote:
dw_mmc driver was responding to errors with debug(). Change that to
prinf so that any errors are immediately obvious. Also adjust english
in comments.
Signed-off-by: Pavel Machek pa...@denx.de
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 0df30bc..4c16e7f 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -177,14 +177,16 @@ static int dwmci_send_cmd(struct mmc *mmc, struct
mmc_cmd *cmd,
}
}
- if (i == retry)
+ if (i == retry) {
+ printf(dwmci_send_cmd: timeout..\n);
return TIMEOUT;
+ }
if (mask DWMCI_INTMSK_RTO) {
- debug(Response Timeout..\n);
+ printf(dwmci_send_cmd: Response Timeout..\n);
return TIMEOUT;
} else if (mask DWMCI_INTMSK_RE) {
- debug(Response Error..\n);
+ printf(dwmci_send_cmd: Response Error..\n);
return -1;
}
@@ -204,7 +206,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd
*cmd,
do {
mask = dwmci_readl(host, DWMCI_RINTSTS);
if (mask (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) {
- debug(DATA ERROR!\n);
+ printf(dwmci_send_cmd: DATA ERROR!\n);
return -1;
}
} while (!(mask DWMCI_INTMSK_DTO));
@@ -232,16 +234,16 @@ static int dwmci_setup_bus(struct dwmci_host *host, u32
freq)
if ((freq == host-clock) || (freq == 0))
return 0;
/*
- * If host-get_mmc_clk didn't define,
+ * If host-get_mmc_clk isn't defined,
* then assume that host-bus_hz is source clock value.
- * host-bus_hz should be set from user.
+ * host-bus_hz should be set by user.
*/
if (host-get_mmc_clk)
sclk = host-get_mmc_clk(host);
else if (host-bus_hz)
sclk = host-bus_hz;
else {
- printf(Didn't get source clock value..\n);
+ printf(dwmci_setup_bus: Didn't get source clock value..\n);
return -EINVAL;
}
@@ -260,7 +262,7 @@ static int dwmci_setup_bus(struct dwmci_host *host, u32
freq)
do {
status = dwmci_readl(host, DWMCI_CMD);
if (timeout-- 0) {
- printf(TIMEOUT error!!\n);
+ printf(dwmci_setup_bus: timeout!\n);
return -ETIMEDOUT;
}
} while (status DWMCI_CMD_START);
@@ -275,7 +277,7 @@ static int dwmci_setup_bus(struct dwmci_host *host, u32
freq)
do {
status = dwmci_readl(host, DWMCI_CMD);
if (timeout-- 0) {
- printf(TIMEOUT error!!\n);
+ printf(dwmci_setup_bus: timeout!\n);
According to your the main purpose, this patch is goal that noticed where/what
error is occurred.
Then i think good that add the __LINE__.
Because there are same message like dwmci_setup_bus: tiemout!\n at some place.
Then i think we don't need to discuss about this patch with Marek. :)
Best Regards,
Jaehoon Chung
return -ETIMEDOUT;
}
} while (status DWMCI_CMD_START);
@@ -290,7 +292,7 @@ static void dwmci_set_ios(struct mmc *mmc)
struct dwmci_host *host = (struct dwmci_host *)mmc-priv;
u32 ctype, regs;
- debug(Buswidth = %d, clock: %d\n,mmc-bus_width, mmc-clock);
+ debug(Buswidth = %d, clock: %d\n, mmc-bus_width, mmc-clock);
dwmci_setup_bus(host, mmc-clock);
switch (mmc-bus_width) {
@@ -329,7 +331,7 @@ static int dwmci_init(struct mmc *mmc)
dwmci_writel(host, DWMCI_PWREN, 1);
if (!dwmci_wait_reset(host, DWMCI_RESET_ALL)) {
- debug(%s[%d] Fail-reset!!\n,__func__,__LINE__);
+ printf(%s[%d] Fail-reset!!\n, __func__, __LINE__);
return -1;
}
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] dw_mmc: cleanups
dw_mmc driver was responding to errors with debug(). Change that to
prinf so that any errors are immediately obvious. Also adjust english
in comments.
Signed-off-by: Pavel Machek pa...@denx.de
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 0df30bc..4c16e7f 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -177,14 +177,16 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd
*cmd,
}
}
- if (i == retry)
+ if (i == retry) {
+ printf(dwmci_send_cmd: timeout..\n);
return TIMEOUT;
+ }
if (mask DWMCI_INTMSK_RTO) {
- debug(Response Timeout..\n);
+ printf(dwmci_send_cmd: Response Timeout..\n);
return TIMEOUT;
} else if (mask DWMCI_INTMSK_RE) {
- debug(Response Error..\n);
+ printf(dwmci_send_cmd: Response Error..\n);
return -1;
}
@@ -204,7 +206,7 @@ static int dwmci_send_cmd(struct mmc *mmc, struct mmc_cmd
*cmd,
do {
mask = dwmci_readl(host, DWMCI_RINTSTS);
if (mask (DWMCI_DATA_ERR | DWMCI_DATA_TOUT)) {
- debug(DATA ERROR!\n);
+ printf(dwmci_send_cmd: DATA ERROR!\n);
return -1;
}
} while (!(mask DWMCI_INTMSK_DTO));
@@ -232,16 +234,16 @@ static int dwmci_setup_bus(struct dwmci_host *host, u32
freq)
if ((freq == host-clock) || (freq == 0))
return 0;
/*
-* If host-get_mmc_clk didn't define,
+* If host-get_mmc_clk isn't defined,
* then assume that host-bus_hz is source clock value.
-* host-bus_hz should be set from user.
+* host-bus_hz should be set by user.
*/
if (host-get_mmc_clk)
sclk = host-get_mmc_clk(host);
else if (host-bus_hz)
sclk = host-bus_hz;
else {
- printf(Didn't get source clock value..\n);
+ printf(dwmci_setup_bus: Didn't get source clock value..\n);
return -EINVAL;
}
@@ -260,7 +262,7 @@ static int dwmci_setup_bus(struct dwmci_host *host, u32
freq)
do {
status = dwmci_readl(host, DWMCI_CMD);
if (timeout-- 0) {
- printf(TIMEOUT error!!\n);
+ printf(dwmci_setup_bus: timeout!\n);
return -ETIMEDOUT;
}
} while (status DWMCI_CMD_START);
@@ -275,7 +277,7 @@ static int dwmci_setup_bus(struct dwmci_host *host, u32
freq)
do {
status = dwmci_readl(host, DWMCI_CMD);
if (timeout-- 0) {
- printf(TIMEOUT error!!\n);
+ printf(dwmci_setup_bus: timeout!\n);
return -ETIMEDOUT;
}
} while (status DWMCI_CMD_START);
@@ -290,7 +292,7 @@ static void dwmci_set_ios(struct mmc *mmc)
struct dwmci_host *host = (struct dwmci_host *)mmc-priv;
u32 ctype, regs;
- debug(Buswidth = %d, clock: %d\n,mmc-bus_width, mmc-clock);
+ debug(Buswidth = %d, clock: %d\n, mmc-bus_width, mmc-clock);
dwmci_setup_bus(host, mmc-clock);
switch (mmc-bus_width) {
@@ -329,7 +331,7 @@ static int dwmci_init(struct mmc *mmc)
dwmci_writel(host, DWMCI_PWREN, 1);
if (!dwmci_wait_reset(host, DWMCI_RESET_ALL)) {
- debug(%s[%d] Fail-reset!!\n,__func__,__LINE__);
+ printf(%s[%d] Fail-reset!!\n, __func__, __LINE__);
return -1;
}
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dw_mmc: cleanups
On Friday, September 05, 2014 at 12:49:48 PM, Pavel Machek wrote:
dw_mmc driver was responding to errors with debug(). Change that to
prinf so that any errors are immediately obvious. Also adjust english
in comments.
Signed-off-by: Pavel Machek pa...@denx.de
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 0df30bc..4c16e7f 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -177,14 +177,16 @@ static int dwmci_send_cmd(struct mmc *mmc, struct
mmc_cmd *cmd, }
}
- if (i == retry)
+ if (i == retry) {
+ printf(dwmci_send_cmd: timeout..\n);
puts(), please fix globally.
[...]
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dw_mmc: cleanups
On Fri 2014-09-05 13:40:58, Marek Vasut wrote:
On Friday, September 05, 2014 at 12:49:48 PM, Pavel Machek wrote:
dw_mmc driver was responding to errors with debug(). Change that to
prinf so that any errors are immediately obvious. Also adjust english
in comments.
Signed-off-by: Pavel Machek pa...@denx.de
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 0df30bc..4c16e7f 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -177,14 +177,16 @@ static int dwmci_send_cmd(struct mmc *mmc, struct
mmc_cmd *cmd, }
}
- if (i == retry)
+ if (i == retry) {
+ printf(dwmci_send_cmd: timeout..\n);
puts(), please fix globally.
Won't.
printf() is canonical way of printing, and is used in such way at 1000
places in u-boot.
But feel free to prepare patchv2 yourself if you think it is worth the
effort.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dw_mmc: cleanups
On Friday, September 05, 2014 at 01:59:19 PM, Pavel Machek wrote:
On Fri 2014-09-05 13:40:58, Marek Vasut wrote:
On Friday, September 05, 2014 at 12:49:48 PM, Pavel Machek wrote:
dw_mmc driver was responding to errors with debug(). Change that to
prinf so that any errors are immediately obvious. Also adjust english
in comments.
Signed-off-by: Pavel Machek pa...@denx.de
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 0df30bc..4c16e7f 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -177,14 +177,16 @@ static int dwmci_send_cmd(struct mmc *mmc, struct
mmc_cmd *cmd, }
}
- if (i == retry)
+ if (i == retry) {
+ printf(dwmci_send_cmd: timeout..\n);
puts(), please fix globally.
Won't.
printf() is canonical way of printing, and is used in such way at 1000
places in u-boot.
The agreement is to use puts() when there are no args attached to the print,
so that the code size is reduced.
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dw_mmc: cleanups
On Fri 2014-09-05 14:05:48, Marek Vasut wrote:
On Friday, September 05, 2014 at 01:59:19 PM, Pavel Machek wrote:
On Fri 2014-09-05 13:40:58, Marek Vasut wrote:
On Friday, September 05, 2014 at 12:49:48 PM, Pavel Machek wrote:
dw_mmc driver was responding to errors with debug(). Change that to
prinf so that any errors are immediately obvious. Also adjust english
in comments.
Signed-off-by: Pavel Machek pa...@denx.de
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 0df30bc..4c16e7f 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -177,14 +177,16 @@ static int dwmci_send_cmd(struct mmc *mmc, struct
mmc_cmd *cmd, }
}
- if (i == retry)
+ if (i == retry) {
+ printf(dwmci_send_cmd: timeout..\n);
puts(), please fix globally.
Won't.
printf() is canonical way of printing, and is used in such way at 1000
places in u-boot.
The agreement is to use puts() when there are no args attached to the print,
so that the code size is reduced.
I did not make such agreement with you, and don't think obfuscating
code with printf-puts conversion is a good idea. If there's
codingstyle document I missed somewhere, please point me to it.
If you think global conversion is a good idea, please do it, so that
old code does not serve as a bad example. But I don't think it saves
any size, compared to speed of serial line it does not save any time,
either.
Pavel
PS: Sorry for flames on the mailing list, but marex explicitely asked
me to take discussion there.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
