* [PATCH v4 0/4] Fix debug macros and their usages
@ 2018-08-22 8:33 Nishad Kamdar
2018-08-22 8:34 ` [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG Nishad Kamdar
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Nishad Kamdar @ 2018-08-22 8:33 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Joe Perches, NeilBrown, devel, Christian Lütke-Stetzkamp,
linux-kernel, John Crispin, Dan Carpenter
This patchset fixes the four debug macros N_MSG, ERR_MSG, INIT_MSG and
IRQ_MSG. Each patch fixes one particular macro and its usages.
For N_MSG, replaces printk with dev_<level> without __func__ or __LINE__
or current->comm and current->pid. Removes the do {} while(0) loop for
the single statement macro.
For ERR_MSG and IRQ_MSG, makes the same changes as those for N_MSG, but
further drops the macros and replaces their usages with dev_<level>.
Removes the INIT_MSG macro and its usages.
Changes in v4:
- Create multiple patches, one for each type of macro being
deleted/changed.
Changes in v3:
- Replace usages of ERR_MSG and IRQ_MSG with dev_err() in code itself.
- Remove all INIT_MSG usages.
- Drop ERR_MSG, INIT_MSG and IRQ_MSG from dbg.h.
Changes in v2:
- Replace printk with dev_<level>.
- Remove __func__, __LINE__, current->comm, current->pid from arguments.
- Remove the do {} while(0) loop from these macros.
- Modify commit message to include other changes.
-
Nishad Kamdar (4):
staging: mt7621-mmc: Fix debug macro N_MSG
staging: mt7621-mmc: Fix debug macro ERR_MSG and its usages
staging: mt7621-mmc: Remove macro INIT_MSG and its usages
staging: mt7621-mmc: Fix debug macro IRQ_MSG and its usages
drivers/staging/mt7621-mmc/dbg.h | 36 +------
drivers/staging/mt7621-mmc/sd.c | 180 ++++++++++++++++++++-----------
2 files changed, 121 insertions(+), 95 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG
2018-08-22 8:33 [PATCH v4 0/4] Fix debug macros and their usages Nishad Kamdar
@ 2018-08-22 8:34 ` Nishad Kamdar
2018-08-22 9:09 ` Dan Carpenter
2018-08-22 8:43 ` [PATCH v4 2/4] staging: mt7621-mmc: Fix debug macro ERR_MSG and its usages Nishad Kamdar
` (2 subsequent siblings)
3 siblings, 1 reply; 14+ messages in thread
From: Nishad Kamdar @ 2018-08-22 8:34 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Joe Perches, NeilBrown, devel, Christian Lütke-Stetzkamp,
linux-kernel, John Crispin, Dan Carpenter
This patch fixes the debug macro N_MSG. Replaces printk with
dev_<level> without __func__ or __LINE__ or current->comm and
current->pid. Removes the do {} while(0) loop for the single
statement macro. Issue found by checkpatch.
Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
---
drivers/staging/mt7621-mmc/dbg.h | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
index 2f2c56b73987..c56fb896617a 100644
--- a/drivers/staging/mt7621-mmc/dbg.h
+++ b/drivers/staging/mt7621-mmc/dbg.h
@@ -104,13 +104,10 @@ do { \
#define N_MSG(evt, fmt, args...)
/*
-do { \
- if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
- printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
- host->id, ##args , __FUNCTION__, __LINE__, current->comm, current->pid); \
- } \
-} while(0)
-*/
+ *if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
+ * dev_err(mmc_dev(host->mmc), "%d -> " fmt "\n", host->id, ##args) \
+ *}
+ */
#define ERR_MSG(fmt, args...) \
do { \
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 2/4] staging: mt7621-mmc: Fix debug macro ERR_MSG and its usages
2018-08-22 8:33 [PATCH v4 0/4] Fix debug macros and their usages Nishad Kamdar
2018-08-22 8:34 ` [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG Nishad Kamdar
@ 2018-08-22 8:43 ` Nishad Kamdar
2018-08-22 9:13 ` Dan Carpenter
2018-08-22 8:45 ` [PATCH v4 3/4] staging: mt7621-mmc: Remove macro INIT_MSG " Nishad Kamdar
2018-08-22 8:47 ` [PATCH v4 4/4] staging: mt7621-mmc: Fix debug macro IRQ_MSG " Nishad Kamdar
3 siblings, 1 reply; 14+ messages in thread
From: Nishad Kamdar @ 2018-08-22 8:43 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Joe Perches, NeilBrown, devel, Christian Lütke-Stetzkamp,
linux-kernel, John Crispin, Dan Carpenter
Replace all usages of ERR_MSG with with dev_<level> without __func__
or __LINE__ or current->comm and current->pid. Remove the do {}
while(0) loop for the single statement macro. Drop ERR_MSG from dbg.h.
Issue found by checkpatch.
Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
---
drivers/staging/mt7621-mmc/dbg.h | 6 --
drivers/staging/mt7621-mmc/sd.c | 128 ++++++++++++++++++++++---------
2 files changed, 90 insertions(+), 44 deletions(-)
diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
index c56fb896617a..71295df59ed0 100644
--- a/drivers/staging/mt7621-mmc/dbg.h
+++ b/drivers/staging/mt7621-mmc/dbg.h
@@ -109,12 +109,6 @@ do { \
*}
*/
-#define ERR_MSG(fmt, args...) \
-do { \
- printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
- host->id, ##args, __FUNCTION__, __LINE__, current->comm, current->pid); \
-} while (0);
-
#if 1
//defined CONFIG_MTK_MMC_CD_POLL
#define INIT_MSG(fmt, args...)
diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index 04d23cc7cd4a..6b2c72fc61f2 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -466,7 +466,8 @@ static void msdc_set_mclk(struct msdc_host *host, int ddr, unsigned int hz)
//u8 clksrc = hw->clk_src;
if (!hz) { // set mmc system clock to 0 ?
- //ERR_MSG("set mclk to 0!!!");
+ //dev_err(mmc_dev(host->mmc), "%d -> set mclk to 0!!!\n",
+ // host->id);
msdc_reset_hw(host);
return;
}
@@ -521,7 +522,7 @@ static void msdc_abort_data(struct msdc_host *host)
{
struct mmc_command *stop = host->mrq->stop;
- ERR_MSG("Need to Abort.");
+ dev_err(mmc_dev(host->mmc), "%d -> Need to Abort.\n", host->id);
msdc_reset_hw(host);
msdc_clr_fifo(host);
@@ -530,7 +531,8 @@ static void msdc_abort_data(struct msdc_host *host)
// need to check FIFO count 0 ?
if (stop) { /* try to stop, but may not success */
- ERR_MSG("stop when abort CMD<%d>", stop->opcode);
+ dev_err(mmc_dev(host->mmc), "%d -> stop when abort CMD<%d>\n",
+ host->id, stop->opcode);
(void)msdc_do_command(host, stop, 0, CMD_TIMEOUT);
}
@@ -688,13 +690,17 @@ static void msdc_pm(pm_message_t state, void *data)
} else if (evt == PM_EVENT_RESUME || evt == PM_EVENT_USER_RESUME) {
if (!host->suspend) {
- //ERR_MSG("warning: already resume");
+ //dev_err(mmc_dev(host->mmc),
+ // "%d -> warning: already resume\n",
+ // host->id);
return;
}
/* No PM resume when USR suspend */
if (evt == PM_EVENT_RESUME && host->pm_state.event == PM_EVENT_USER_SUSPEND) {
- ERR_MSG("PM Resume when in USR Suspend"); /* won't happen. */
+ dev_err(mmc_dev(host->mmc),
+ "%d -> PM Resume when in USR Suspend\n",
+ host->id); /* won't happen. */
return;
}
@@ -812,7 +818,9 @@ static unsigned int msdc_command_start(struct msdc_host *host,
break;
if (time_after(jiffies, tmo)) {
- ERR_MSG("XXX cmd_busy timeout: before CMD<%d>", opcode);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> XXX cmd_busy timeout: before CMD<%d>\n",
+ host->id, opcode);
cmd->error = -ETIMEDOUT;
msdc_reset_hw(host);
goto end;
@@ -823,7 +831,9 @@ static unsigned int msdc_command_start(struct msdc_host *host,
if (!sdc_is_busy())
break;
if (time_after(jiffies, tmo)) {
- ERR_MSG("XXX sdc_busy timeout: before CMD<%d>", opcode);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> XXX sdc_busy timeout: before CMD<%d>\n",
+ host->id, opcode);
cmd->error = -ETIMEDOUT;
msdc_reset_hw(host);
goto end;
@@ -862,7 +872,9 @@ static unsigned int msdc_command_resp(struct msdc_host *host,
spin_unlock(&host->lock);
if (!wait_for_completion_timeout(&host->cmd_done, 10 * timeout)) {
- ERR_MSG("XXX CMD<%d> wait_for_completion timeout ARG<0x%.8x>", opcode, cmd->arg);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> XXX CMD<%d> wait_for_completion timeout ARG<0x%.8x>\n",
+ host->id, opcode, cmd->arg);
cmd->error = -ETIMEDOUT;
msdc_reset_hw(host);
}
@@ -1125,15 +1137,22 @@ static int msdc_do_request(struct mmc_host *mmc, struct mmc_request *mrq)
spin_unlock(&host->lock);
if (!wait_for_completion_timeout(&host->xfer_done, DAT_TIMEOUT)) {
- ERR_MSG("XXX CMD<%d> wait xfer_done<%d> timeout!!", cmd->opcode, data->blocks * data->blksz);
- ERR_MSG(" DMA_SA = 0x%x",
- readl(host->base + MSDC_DMA_SA));
- ERR_MSG(" DMA_CA = 0x%x",
- readl(host->base + MSDC_DMA_CA));
- ERR_MSG(" DMA_CTRL = 0x%x",
- readl(host->base + MSDC_DMA_CTRL));
- ERR_MSG(" DMA_CFG = 0x%x",
- readl(host->base + MSDC_DMA_CFG));
+ dev_err(mmc_dev(host->mmc),
+ "%d -> XXX CMD<%d> wait xfer_done<%d> timeout!!\n",
+ host->id, cmd->opcode,
+ data->blocks * data->blksz);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> DMA_SA = 0x%x\n",
+ host->id, readl(host->base + MSDC_DMA_SA));
+ dev_err(mmc_dev(host->mmc),
+ "%d -> DMA_CA = 0x%x\n",
+ host->id, readl(host->base + MSDC_DMA_CA));
+ dev_err(mmc_dev(host->mmc),
+ "%d -> DMA_CTRL = 0x%x\n",
+ host->id, readl(host->base + MSDC_DMA_CTRL));
+ dev_err(mmc_dev(host->mmc),
+ "%d -> DMA_CFG = 0x%x\n",
+ host->id, readl(host->base + MSDC_DMA_CFG));
data->error = -ETIMEDOUT;
msdc_reset_hw(host);
@@ -1200,7 +1219,10 @@ static int msdc_do_request(struct mmc_host *mmc, struct mmc_request *mrq)
if (mrq->stop && mrq->stop->error)
host->error |= 0x100;
- //if (host->error) ERR_MSG("host->error<%d>", host->error);
+ //if (host->error)
+ // dev_err(mmc_dev(host->mmc),
+ // "%d -> host->error<%d>\n",
+ // host->id, host->error);
return host->error;
}
@@ -1260,19 +1282,27 @@ static int msdc_tune_cmdrsp(struct msdc_host *host, struct mmc_command *cmd)
if (host->app_cmd) {
result = msdc_app_cmd(host->mmc, host);
if (result) {
- ERR_MSG("TUNE_CMD app_cmd<%d> failed: RESP_RXDLY<%d>,R_SMPL<%d>",
- host->mrq->cmd->opcode, cur_rrdly, cur_rsmpl);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> TUNE_CMD app_cmd<%d> failed: RESP_RXDLY<%d>,R_SMPL<%d>\n",
+ host->id,
+ host->mrq->cmd->opcode,
+ cur_rrdly, cur_rsmpl);
continue;
}
}
result = msdc_do_command(host, cmd, 0, CMD_TIMEOUT); // not tune.
- ERR_MSG("TUNE_CMD<%d> %s PAD_CMD_RESP_RXDLY[26:22]<%d> R_SMPL[1]<%d>", cmd->opcode,
- (result == 0) ? "PASS" : "FAIL", cur_rrdly, cur_rsmpl);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> TUNE_CMD<%d> %s PAD_CMD_RESP_RXDLY[26:22]<%d> R_SMPL[1]<%d>\n",
+ host->id, cmd->opcode,
+ (result == 0) ? "PASS" : "FAIL", cur_rrdly,
+ cur_rsmpl);
if (result == 0)
return 0;
if (result != -EIO) {
- ERR_MSG("TUNE_CMD<%d> Error<%d> not -EIO", cmd->opcode, result);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> TUNE_CMD<%d> Error<%d> not -EIO\n",
+ host->id, cmd->opcode, result);
return result;
}
@@ -1325,7 +1355,10 @@ static int msdc_tune_bread(struct mmc_host *mmc, struct mmc_request *mrq)
if (host->app_cmd) {
result = msdc_app_cmd(host->mmc, host);
if (result) {
- ERR_MSG("TUNE_BREAD app_cmd<%d> failed", host->mrq->cmd->opcode);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> TUNE_BREAD app_cmd<%d> failed\n",
+ host->id,
+ host->mrq->cmd->opcode);
continue;
}
}
@@ -1336,10 +1369,13 @@ static int msdc_tune_bread(struct mmc_host *mmc, struct mmc_request *mrq)
&dcrc); /* RO */
if (!ddr)
dcrc &= ~SDC_DCRC_STS_NEG;
- ERR_MSG("TUNE_BREAD<%s> dcrc<0x%x> DATRDDLY0/1<0x%x><0x%x> dsmpl<0x%x>",
- (result == 0 && dcrc == 0) ? "PASS" : "FAIL", dcrc,
- readl(host->base + MSDC_DAT_RDDLY0),
- readl(host->base + MSDC_DAT_RDDLY1), cur_dsmpl);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> TUNE_BREAD<%s> dcrc<0x%x> DATRDDLY0/1<0x%x><0x%x> dsmpl<0x%x>\n",
+ host->id,
+ (result == 0 && dcrc == 0) ? "PASS" : "FAIL",
+ dcrc, readl(host->base + MSDC_DAT_RDDLY0),
+ readl(host->base + MSDC_DAT_RDDLY1),
+ cur_dsmpl);
/* Fix me: result is 0, but dcrc is still exist */
if (result == 0 && dcrc == 0) {
@@ -1348,8 +1384,11 @@ static int msdc_tune_bread(struct mmc_host *mmc, struct mmc_request *mrq)
/* there is a case: command timeout, and data phase not processed */
if (mrq->data->error != 0 &&
mrq->data->error != -EIO) {
- ERR_MSG("TUNE_READ: result<0x%x> cmd_error<%d> data_error<%d>",
- result, mrq->cmd->error, mrq->data->error);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> TUNE_READ: result<0x%x> cmd_error<%d> data_error<%d>\n",
+ host->id, result,
+ mrq->cmd->error,
+ mrq->data->error);
goto done;
}
}
@@ -1458,13 +1497,18 @@ static int msdc_tune_bwrite(struct mmc_host *mmc, struct mmc_request *mrq)
if (host->app_cmd) {
result = msdc_app_cmd(host->mmc, host);
if (result) {
- ERR_MSG("TUNE_BWRITE app_cmd<%d> failed", host->mrq->cmd->opcode);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> TUNE_BWRITE app_cmd<%d> failed\n",
+ host->id,
+ host->mrq->cmd->opcode);
continue;
}
}
result = msdc_do_request(mmc, mrq);
- ERR_MSG("TUNE_BWRITE<%s> DSPL<%d> DATWRDLY<%d> MSDC_DAT_RDDLY0<0x%x>",
+ dev_err(mmc_dev(host->mmc),
+ "%d -> TUNE_BWRITE<%s> DSPL<%d> DATWRDLY<%d> MSDC_DAT_RDDLY0<0x%x>\n",
+ host->id,
result == 0 ? "PASS" : "FAIL",
cur_dsmpl, cur_wrrdly, cur_rxdly0);
@@ -1473,8 +1517,11 @@ static int msdc_tune_bwrite(struct mmc_host *mmc, struct mmc_request *mrq)
} else {
/* there is a case: command timeout, and data phase not processed */
if (mrq->data->error != -EIO) {
- ERR_MSG("TUNE_READ: result<0x%x> cmd_error<%d> data_error<%d>",
- result, mrq->cmd->error, mrq->data->error);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> TUNE_READ: result<0x%x> cmd_error<%d> data_error<%d>\n",
+ host->id, result,
+ mrq->cmd->error,
+ mrq->data->error);
goto done;
}
}
@@ -1508,7 +1555,8 @@ static int msdc_get_card_status(struct mmc_host *mmc, struct msdc_host *host, u3
if (mmc->card) {
cmd.arg = mmc->card->rca << 16;
} else {
- ERR_MSG("cmd13 mmc card is null");
+ dev_err(mmc_dev(host->mmc), "%d -> cmd13 mmc card is null\n",
+ host->id);
cmd.arg = host->app_cmd_arg;
}
cmd.flags = MMC_RSP_SPI_R2 | MMC_RSP_R1 | MMC_CMD_AC;
@@ -1535,7 +1583,8 @@ static int msdc_check_busy(struct mmc_host *mmc, struct msdc_host *host)
if (err)
return err;
/* need cmd12? */
- ERR_MSG("cmd<13> resp<0x%x>", status);
+ dev_err(mmc_dev(host->mmc), "%d -> cmd<13> resp<0x%x>\n",
+ host->id, status);
} while (R1_CURRENT_STATE(status) == 7);
return err;
@@ -1559,7 +1608,9 @@ static int msdc_tune_request(struct mmc_host *mmc, struct mmc_request *mrq)
} else {
ret = msdc_check_busy(mmc, host);
if (ret) {
- ERR_MSG("XXX cmd13 wait program done failed");
+ dev_err(mmc_dev(host->mmc),
+ "%d -> XXX cmd13 wait program done failed\n",
+ host->id);
return ret;
}
/* CRC and TO */
@@ -2289,7 +2340,8 @@ static int msdc_drv_remove(struct platform_device *pdev)
host = mmc_priv(mmc);
BUG_ON(!host);
- ERR_MSG("removed !!!");
+ dev_err(mmc_dev(host->mmc), "%d -> removed !!!\n",
+ host->id);
platform_set_drvdata(pdev, NULL);
mmc_remove_host(host->mmc);
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 3/4] staging: mt7621-mmc: Remove macro INIT_MSG and its usages
2018-08-22 8:33 [PATCH v4 0/4] Fix debug macros and their usages Nishad Kamdar
2018-08-22 8:34 ` [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG Nishad Kamdar
2018-08-22 8:43 ` [PATCH v4 2/4] staging: mt7621-mmc: Fix debug macro ERR_MSG and its usages Nishad Kamdar
@ 2018-08-22 8:45 ` Nishad Kamdar
2018-08-22 8:47 ` [PATCH v4 4/4] staging: mt7621-mmc: Fix debug macro IRQ_MSG " Nishad Kamdar
3 siblings, 0 replies; 14+ messages in thread
From: Nishad Kamdar @ 2018-08-22 8:45 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Joe Perches, NeilBrown, devel, Christian Lütke-Stetzkamp,
linux-kernel, John Crispin, Dan Carpenter
Removed all usages of INIT_MSG and dropped it from dbg.h.
Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
---
drivers/staging/mt7621-mmc/dbg.h | 7 -------
drivers/staging/mt7621-mmc/sd.c | 16 ----------------
2 files changed, 23 deletions(-)
diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
index 71295df59ed0..8d2c16450ef5 100644
--- a/drivers/staging/mt7621-mmc/dbg.h
+++ b/drivers/staging/mt7621-mmc/dbg.h
@@ -111,15 +111,8 @@ do { \
#if 1
//defined CONFIG_MTK_MMC_CD_POLL
-#define INIT_MSG(fmt, args...)
#define IRQ_MSG(fmt, args...)
#else
-#define INIT_MSG(fmt, args...) \
-do { \
- printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
- host->id, ##args, __FUNCTION__, __LINE__, current->comm, current->pid); \
-} while (0);
-
/* PID in ISR in not corrent */
#define IRQ_MSG(fmt, args...) \
do { \
diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index 6b2c72fc61f2..327c1cd7fd04 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -187,12 +187,10 @@ static u32 hclks[] = {50000000}; /* +/- by chhung */
//============================================
#define msdc_vcore_on(host) \
do { \
- INIT_MSG("[+]VMC ref. count<%d>", ++host->pwr_ref); \
(void)hwPowerOn(MT65XX_POWER_LDO_VMC, VOL_3300, "SD"); \
} while (0)
#define msdc_vcore_off(host) \
do { \
- INIT_MSG("[-]VMC ref. count<%d>", --host->pwr_ref); \
(void)hwPowerDown(MT65XX_POWER_LDO_VMC, "SD"); \
} while (0)
@@ -439,7 +437,6 @@ static void msdc_select_clksrc(struct msdc_host *host, unsigned char clksrc)
u32 val;
BUG_ON(clksrc > 3);
- INIT_MSG("set clock source to <%d>", clksrc);
val = readl(host->base + MSDC_CLKSRC_REG);
if (readl(host->base + MSDC_ECO_VER) >= 4) {
@@ -510,10 +507,6 @@ static void msdc_set_mclk(struct msdc_host *host, int ddr, unsigned int hz)
host->mclk = hz;
msdc_set_timeout(host, host->timeout_ns, host->timeout_clks); // need?
- INIT_MSG("================");
- INIT_MSG("!!! Set<%dKHz> Source<%dKHz> -> sclk<%dKHz>", hz / 1000, hclk / 1000, sclk / 1000);
- INIT_MSG("================");
-
msdc_irq_restore(flags);
}
@@ -671,12 +664,6 @@ static void msdc_pm(pm_message_t state, void *data)
struct msdc_host *host = (struct msdc_host *)data;
int evt = state.event;
- if (evt == PM_EVENT_USER_RESUME || evt == PM_EVENT_USER_SUSPEND) {
- INIT_MSG("USR_%s: suspend<%d> power<%d>",
- evt == PM_EVENT_USER_RESUME ? "EVENT_USER_RESUME" : "EVENT_USER_SUSPEND",
- host->suspend, host->power_mode);
- }
-
if (evt == PM_EVENT_SUSPEND || evt == PM_EVENT_USER_SUSPEND) {
if (host->suspend) /* already suspend */ /* default 0*/
return;
@@ -1762,7 +1749,6 @@ static void msdc_ops_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
if (host->mclk != ios->clock) {
if (ios->clock > 25000000) {
//if (!(host->hw->flags & MSDC_REMOVABLE)) {
- INIT_MSG("SD data latch edge<%d>", MSDC_SMPL_FALLING);
sdr_set_field(host->base + MSDC_IOCON, MSDC_IOCON_RSPL,
MSDC_SMPL_FALLING);
sdr_set_field(host->base + MSDC_IOCON, MSDC_IOCON_DSPL,
@@ -1815,7 +1801,6 @@ static int msdc_ops_get_cd(struct mmc_host *mmc)
return 1;
#else
host->card_inserted = (host->pm_state.event == PM_EVENT_USER_RESUME) ? 1 : 0;
- INIT_MSG("sdio ops_get_cd<%d>", host->card_inserted);
return host->card_inserted;
#endif
}
@@ -1839,7 +1824,6 @@ static int msdc_ops_get_cd(struct mmc_host *mmc)
present = 0; /* TODO? Check DAT3 pins for card detection */
}
- INIT_MSG("ops_get_cd return<%d>", present);
return present;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 4/4] staging: mt7621-mmc: Fix debug macro IRQ_MSG and its usages
2018-08-22 8:33 [PATCH v4 0/4] Fix debug macros and their usages Nishad Kamdar
` (2 preceding siblings ...)
2018-08-22 8:45 ` [PATCH v4 3/4] staging: mt7621-mmc: Remove macro INIT_MSG " Nishad Kamdar
@ 2018-08-22 8:47 ` Nishad Kamdar
3 siblings, 0 replies; 14+ messages in thread
From: Nishad Kamdar @ 2018-08-22 8:47 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Joe Perches, NeilBrown, devel, Christian Lütke-Stetzkamp,
linux-kernel, John Crispin, Dan Carpenter
Replace all usages of IRQ_MSG with with dev_<level> without __func__
or __LINE__ or current->comm and current->pid. Remove the do {}
while(0) loop for the single statement macro. Drop IRQ_MSG from dbg.h.
Issue found by checkpatch.
Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
---
drivers/staging/mt7621-mmc/dbg.h | 12 -----------
drivers/staging/mt7621-mmc/sd.c | 36 ++++++++++++++++++++++++--------
2 files changed, 27 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
index 8d2c16450ef5..5458dae5dc03 100644
--- a/drivers/staging/mt7621-mmc/dbg.h
+++ b/drivers/staging/mt7621-mmc/dbg.h
@@ -109,18 +109,6 @@ do { \
*}
*/
-#if 1
-//defined CONFIG_MTK_MMC_CD_POLL
-#define IRQ_MSG(fmt, args...)
-#else
-/* PID in ISR in not corrent */
-#define IRQ_MSG(fmt, args...) \
-do { \
- printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d>\n", \
- host->id, ##args, __FUNCTION__, __LINE__); \
-} while (0);
-#endif
-
void msdc_debug_proc_init(void);
#if 0 /* --- chhung */
diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
index 327c1cd7fd04..5cb6bc36e78b 100644
--- a/drivers/staging/mt7621-mmc/sd.c
+++ b/drivers/staging/mt7621-mmc/sd.c
@@ -420,7 +420,9 @@ static void msdc_tasklet_card(struct work_struct *work)
mmc_detect_change(host->mmc, msecs_to_jiffies(20));
}
- IRQ_MSG("card found<%s>", inserted ? "inserted" : "removed");
+ dev_err(mmc_dev(host->mmc),
+ "%d -> card found<%s>\n",
+ host->id, inserted ? "inserted" : "removed");
#endif
spin_unlock(&host->lock);
@@ -1858,14 +1860,17 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
if (intsts & MSDC_INT_CDSC) {
if (host->mmc->caps & MMC_CAP_NEEDS_POLL)
return IRQ_HANDLED;
- IRQ_MSG("MSDC_INT_CDSC irq<0x%.8x>", intsts);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> MSDC_INT_CDSC irq<0x%.8x>\n", host->id, intsts);
schedule_delayed_work(&host->card_delaywork, HZ);
/* tuning when plug card ? */
}
/* sdio interrupt */
if (intsts & MSDC_INT_SDIOIRQ) {
- IRQ_MSG("XXX MSDC_INT_SDIOIRQ"); /* seems not sdio irq */
+ dev_err(mmc_dev(host->mmc),
+ "%d -> XXX MSDC_INT_SDIOIRQ\n",
+ host->id); /* seems not sdio irq */
//mmc_signal_sdio_irq(host->mmc);
}
@@ -1883,10 +1888,15 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
msdc_clr_int();
if (intsts & MSDC_INT_DATTMO) {
- IRQ_MSG("XXX CMD<%d> MSDC_INT_DATTMO", host->mrq->cmd->opcode);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> XXX CMD<%d> MSDC_INT_DATTMO\n",
+ host->id, host->mrq->cmd->opcode);
data->error = -ETIMEDOUT;
} else if (intsts & MSDC_INT_DATCRCERR) {
- IRQ_MSG("XXX CMD<%d> MSDC_INT_DATCRCERR, SDC_DCRC_STS<0x%x>", host->mrq->cmd->opcode, readl(host->base + SDC_DCRC_STS));
+ dev_err(mmc_dev(host->mmc),
+ "%d -> XXX CMD<%d> MSDC_INT_DATCRCERR, SDC_DCRC_STS<0x%x>\n",
+ host->id, host->mrq->cmd->opcode,
+ readl(host->base + SDC_DCRC_STS);
data->error = -EIO;
}
@@ -1919,15 +1929,23 @@ static irqreturn_t msdc_irq(int irq, void *dev_id)
}
} else if ((intsts & MSDC_INT_RSPCRCERR) || (intsts & MSDC_INT_ACMDCRCERR)) {
if (intsts & MSDC_INT_ACMDCRCERR)
- IRQ_MSG("XXX CMD<%d> MSDC_INT_ACMDCRCERR", cmd->opcode);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> XXX CMD<%d> MSDC_INT_ACMDCRCERR\n",
+ host->id, cmd->opcode);
else
- IRQ_MSG("XXX CMD<%d> MSDC_INT_RSPCRCERR", cmd->opcode);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> XXX CMD<%d> MSDC_INT_RSPCRCERR\n",
+ host->id, cmd->opcode);
cmd->error = -EIO;
} else if ((intsts & MSDC_INT_CMDTMO) || (intsts & MSDC_INT_ACMDTMO)) {
if (intsts & MSDC_INT_ACMDTMO)
- IRQ_MSG("XXX CMD<%d> MSDC_INT_ACMDTMO", cmd->opcode);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> XXX CMD<%d> MSDC_INT_ACMDTMO\n",
+ host->id, cmd->opcode);
else
- IRQ_MSG("XXX CMD<%d> MSDC_INT_CMDTMO", cmd->opcode);
+ dev_err(mmc_dev(host->mmc),
+ "%d -> XXX CMD<%d> MSDC_INT_CMDTMO\n",
+ host->id, cmd->opcode);
cmd->error = -ETIMEDOUT;
msdc_reset_hw(host);
msdc_clr_fifo(host);
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG
2018-08-22 8:34 ` [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG Nishad Kamdar
@ 2018-08-22 9:09 ` Dan Carpenter
2018-08-22 11:10 ` Nishad Kamdar
0 siblings, 1 reply; 14+ messages in thread
From: Dan Carpenter @ 2018-08-22 9:09 UTC (permalink / raw)
To: Nishad Kamdar
Cc: Greg Kroah-Hartman, devel, linux-kernel, NeilBrown, Joe Perches,
Christian Lütke-Stetzkamp, John Crispin
On Wed, Aug 22, 2018 at 02:04:55PM +0530, Nishad Kamdar wrote:
> This patch fixes the debug macro N_MSG. Replaces printk with
> dev_<level> without __func__ or __LINE__ or current->comm and
> current->pid. Removes the do {} while(0) loop for the single
> statement macro. Issue found by checkpatch.
>
> Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
> ---
> drivers/staging/mt7621-mmc/dbg.h | 11 ++++-------
> 1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
> index 2f2c56b73987..c56fb896617a 100644
> --- a/drivers/staging/mt7621-mmc/dbg.h
> +++ b/drivers/staging/mt7621-mmc/dbg.h
> @@ -104,13 +104,10 @@ do { \
>
> #define N_MSG(evt, fmt, args...)
> /*
> -do { \
> - if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> - printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
> - host->id, ##args , __FUNCTION__, __LINE__, current->comm, current->pid); \
> - } \
> -} while(0)
> -*/
> + *if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> + * dev_err(mmc_dev(host->mmc), "%d -> " fmt "\n", host->id, ##args) \
> + *}
> + */
I don't understand what you're trying to do here. You just commented
out the macro and turned it into a no-op. That's not what the patch
description says.
To me the original code seems fine.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/4] staging: mt7621-mmc: Fix debug macro ERR_MSG and its usages
2018-08-22 8:43 ` [PATCH v4 2/4] staging: mt7621-mmc: Fix debug macro ERR_MSG and its usages Nishad Kamdar
@ 2018-08-22 9:13 ` Dan Carpenter
2018-08-22 11:16 ` Nishad Kamdar
2018-08-23 17:15 ` Nishad Kamdar
0 siblings, 2 replies; 14+ messages in thread
From: Dan Carpenter @ 2018-08-22 9:13 UTC (permalink / raw)
To: Nishad Kamdar
Cc: Greg Kroah-Hartman, devel, linux-kernel, NeilBrown, Joe Perches,
Christian Lütke-Stetzkamp, John Crispin
On Wed, Aug 22, 2018 at 02:13:07PM +0530, Nishad Kamdar wrote:
> diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
> index 04d23cc7cd4a..6b2c72fc61f2 100644
> --- a/drivers/staging/mt7621-mmc/sd.c
> +++ b/drivers/staging/mt7621-mmc/sd.c
> @@ -466,7 +466,8 @@ static void msdc_set_mclk(struct msdc_host *host, int ddr, unsigned int hz)
> //u8 clksrc = hw->clk_src;
>
> if (!hz) { // set mmc system clock to 0 ?
> - //ERR_MSG("set mclk to 0!!!");
> + //dev_err(mmc_dev(host->mmc), "%d -> set mclk to 0!!!\n",
> + // host->id);
Just delete commented out code.
> msdc_reset_hw(host);
> return;
> }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG
2018-08-22 9:09 ` Dan Carpenter
@ 2018-08-22 11:10 ` Nishad Kamdar
2018-08-22 11:26 ` Greg Kroah-Hartman
2018-08-22 11:38 ` Dan Carpenter
0 siblings, 2 replies; 14+ messages in thread
From: Nishad Kamdar @ 2018-08-22 11:10 UTC (permalink / raw)
To: Dan Carpenter, Greg Kroah-Hartman
Cc: devel, linux-kernel, NeilBrown, Joe Perches,
Christian Lütke-Stetzkamp, John Crispin
On Wed, Aug 22, 2018 at 12:09:36PM +0300, Dan Carpenter wrote:
> On Wed, Aug 22, 2018 at 02:04:55PM +0530, Nishad Kamdar wrote:
> > This patch fixes the debug macro N_MSG. Replaces printk with
> > dev_<level> without __func__ or __LINE__ or current->comm and
> > current->pid. Removes the do {} while(0) loop for the single
> > statement macro. Issue found by checkpatch.
> >
> > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
> > ---
> > drivers/staging/mt7621-mmc/dbg.h | 11 ++++-------
> > 1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
> > index 2f2c56b73987..c56fb896617a 100644
> > --- a/drivers/staging/mt7621-mmc/dbg.h
> > +++ b/drivers/staging/mt7621-mmc/dbg.h
> > @@ -104,13 +104,10 @@ do { \
> >
> > #define N_MSG(evt, fmt, args...)
> > /*
> > -do { \
> > - if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > - printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
> > - host->id, ##args , __FUNCTION__, __LINE__, current->comm, current->pid); \
> > - } \
> > -} while(0)
> > -*/
> > + *if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > + * dev_err(mmc_dev(host->mmc), "%d -> " fmt "\n", host->id, ##args) \
> > + *}
> > + */
>
> I don't understand what you're trying to do here. You just commented
> out the macro and turned it into a no-op. That's not what the patch
> description says.
>
> To me the original code seems fine.
>
> regards,
> dan carpenter
>
Ok. But the macro definition was already commented.
I just changed printk to dev_err and gave it a better
block comment format.
Should I leave the older version as it is ?
Thanks for the review.
regards,
nishad
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/4] staging: mt7621-mmc: Fix debug macro ERR_MSG and its usages
2018-08-22 9:13 ` Dan Carpenter
@ 2018-08-22 11:16 ` Nishad Kamdar
2018-08-23 17:15 ` Nishad Kamdar
1 sibling, 0 replies; 14+ messages in thread
From: Nishad Kamdar @ 2018-08-22 11:16 UTC (permalink / raw)
To: i Dan Carpenter, Greg Kroah-Hartman
Cc: devel, linux-kernel, NeilBrown, Joe Perches,
Christian Lütke-Stetzkamp, John Crispin
On Wed, Aug 22, 2018 at 12:13:42PM +0300, Dan Carpenter wrote:
> On Wed, Aug 22, 2018 at 02:13:07PM +0530, Nishad Kamdar wrote:
> > diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
> > index 04d23cc7cd4a..6b2c72fc61f2 100644
> > --- a/drivers/staging/mt7621-mmc/sd.c
> > +++ b/drivers/staging/mt7621-mmc/sd.c
> > @@ -466,7 +466,8 @@ static void msdc_set_mclk(struct msdc_host *host, int ddr, unsigned int hz)
> > //u8 clksrc = hw->clk_src;
> >
> > if (!hz) { // set mmc system clock to 0 ?
> > - //ERR_MSG("set mclk to 0!!!");
> > + //dev_err(mmc_dev(host->mmc), "%d -> set mclk to 0!!!\n",
> > + // host->id);
>
> Just delete commented out code.
>
> > msdc_reset_hw(host);
> > return;
> > }
>
> regards,
> dan carpenter
>
Ok, I'll do that.
Thanks for the review.
regards,
nishad
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG
2018-08-22 11:10 ` Nishad Kamdar
@ 2018-08-22 11:26 ` Greg Kroah-Hartman
2018-08-23 17:13 ` Nishad Kamdar
2018-08-22 11:38 ` Dan Carpenter
1 sibling, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2018-08-22 11:26 UTC (permalink / raw)
To: Nishad Kamdar
Cc: Dan Carpenter, devel, linux-kernel, Joe Perches, NeilBrown,
Christian Lütke-Stetzkamp, John Crispin
On Wed, Aug 22, 2018 at 04:40:56PM +0530, Nishad Kamdar wrote:
> On Wed, Aug 22, 2018 at 12:09:36PM +0300, Dan Carpenter wrote:
> > On Wed, Aug 22, 2018 at 02:04:55PM +0530, Nishad Kamdar wrote:
> > > This patch fixes the debug macro N_MSG. Replaces printk with
> > > dev_<level> without __func__ or __LINE__ or current->comm and
> > > current->pid. Removes the do {} while(0) loop for the single
> > > statement macro. Issue found by checkpatch.
> > >
> > > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
> > > ---
> > > drivers/staging/mt7621-mmc/dbg.h | 11 ++++-------
> > > 1 file changed, 4 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
> > > index 2f2c56b73987..c56fb896617a 100644
> > > --- a/drivers/staging/mt7621-mmc/dbg.h
> > > +++ b/drivers/staging/mt7621-mmc/dbg.h
> > > @@ -104,13 +104,10 @@ do { \
> > >
> > > #define N_MSG(evt, fmt, args...)
> > > /*
> > > -do { \
> > > - if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > > - printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
> > > - host->id, ##args , __FUNCTION__, __LINE__, current->comm, current->pid); \
> > > - } \
> > > -} while(0)
> > > -*/
> > > + *if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > > + * dev_err(mmc_dev(host->mmc), "%d -> " fmt "\n", host->id, ##args) \
> > > + *}
> > > + */
> >
> > I don't understand what you're trying to do here. You just commented
> > out the macro and turned it into a no-op. That's not what the patch
> > description says.
> >
> > To me the original code seems fine.
> >
> > regards,
> > dan carpenter
> >
>
> Ok. But the macro definition was already commented.
> I just changed printk to dev_err and gave it a better
> block comment format.
> Should I leave the older version as it is ?
No, delete the whole thing if it is not being used, don't "convert" code
that is commented out.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG
2018-08-22 11:10 ` Nishad Kamdar
2018-08-22 11:26 ` Greg Kroah-Hartman
@ 2018-08-22 11:38 ` Dan Carpenter
2018-08-23 17:19 ` Nishad Kamdar
1 sibling, 1 reply; 14+ messages in thread
From: Dan Carpenter @ 2018-08-22 11:38 UTC (permalink / raw)
To: Nishad Kamdar
Cc: Greg Kroah-Hartman, devel, linux-kernel, NeilBrown, Joe Perches,
Christian Lütke-Stetzkamp, John Crispin
On Wed, Aug 22, 2018 at 04:40:56PM +0530, Nishad Kamdar wrote:
> On Wed, Aug 22, 2018 at 12:09:36PM +0300, Dan Carpenter wrote:
> > On Wed, Aug 22, 2018 at 02:04:55PM +0530, Nishad Kamdar wrote:
> > > This patch fixes the debug macro N_MSG. Replaces printk with
> > > dev_<level> without __func__ or __LINE__ or current->comm and
> > > current->pid. Removes the do {} while(0) loop for the single
> > > statement macro. Issue found by checkpatch.
> > >
> > > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
> > > ---
> > > drivers/staging/mt7621-mmc/dbg.h | 11 ++++-------
> > > 1 file changed, 4 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
> > > index 2f2c56b73987..c56fb896617a 100644
> > > --- a/drivers/staging/mt7621-mmc/dbg.h
> > > +++ b/drivers/staging/mt7621-mmc/dbg.h
> > > @@ -104,13 +104,10 @@ do { \
> > >
> > > #define N_MSG(evt, fmt, args...)
> > > /*
> > > -do { \
> > > - if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > > - printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
> > > - host->id, ##args , __FUNCTION__, __LINE__, current->comm, current->pid); \
> > > - } \
> > > -} while(0)
> > > -*/
> > > + *if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > > + * dev_err(mmc_dev(host->mmc), "%d -> " fmt "\n", host->id, ##args) \
> > > + *}
> > > + */
> >
> > I don't understand what you're trying to do here. You just commented
> > out the macro and turned it into a no-op. That's not what the patch
> > description says.
> >
> > To me the original code seems fine.
> >
> > regards,
> > dan carpenter
> >
>
> Ok. But the macro definition was already commented.
> I just changed printk to dev_err and gave it a better
> block comment format.
> Should I leave the older version as it is ?
Oh... Yeah. You're right. Sorry about that.
We normally just delete dead code without even thinking hard about it.
It turns out that if we delete all the N_MSG() code, it's actually quite
a bit. But probably it's the right thing to do, instead of hanging on
to it like a hoarder.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG
2018-08-22 11:26 ` Greg Kroah-Hartman
@ 2018-08-23 17:13 ` Nishad Kamdar
0 siblings, 0 replies; 14+ messages in thread
From: Nishad Kamdar @ 2018-08-23 17:13 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Joe Perches, NeilBrown, devel, Christian Lütke-Stetzkamp,
linux-kernel, John Crispin, Dan Carpenter
On Wed, Aug 22, 2018 at 01:26:41PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Aug 22, 2018 at 04:40:56PM +0530, Nishad Kamdar wrote:
> > On Wed, Aug 22, 2018 at 12:09:36PM +0300, Dan Carpenter wrote:
> > > On Wed, Aug 22, 2018 at 02:04:55PM +0530, Nishad Kamdar wrote:
> > > > This patch fixes the debug macro N_MSG. Replaces printk with
> > > > dev_<level> without __func__ or __LINE__ or current->comm and
> > > > current->pid. Removes the do {} while(0) loop for the single
> > > > statement macro. Issue found by checkpatch.
> > > >
> > > > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
> > > > ---
> > > > drivers/staging/mt7621-mmc/dbg.h | 11 ++++-------
> > > > 1 file changed, 4 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
> > > > index 2f2c56b73987..c56fb896617a 100644
> > > > --- a/drivers/staging/mt7621-mmc/dbg.h
> > > > +++ b/drivers/staging/mt7621-mmc/dbg.h
> > > > @@ -104,13 +104,10 @@ do { \
> > > >
> > > > #define N_MSG(evt, fmt, args...)
> > > > /*
> > > > -do { \
> > > > - if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > > > - printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
> > > > - host->id, ##args , __FUNCTION__, __LINE__, current->comm, current->pid); \
> > > > - } \
> > > > -} while(0)
> > > > -*/
> > > > + *if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > > > + * dev_err(mmc_dev(host->mmc), "%d -> " fmt "\n", host->id, ##args) \
> > > > + *}
> > > > + */
> > >
> > > I don't understand what you're trying to do here. You just commented
> > > out the macro and turned it into a no-op. That's not what the patch
> > > description says.
> > >
> > > To me the original code seems fine.
> > >
> > > regards,
> > > dan carpenter
> > >
> >
> > Ok. But the macro definition was already commented.
> > I just changed printk to dev_err and gave it a better
> > block comment format.
> > Should I leave the older version as it is ?
>
> No, delete the whole thing if it is not being used, don't "convert" code
> that is commented out.
>
> thanks,
>
> greg k-h
Ok, I'll do that.
Thanks for the review.
regards,
nishad
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/4] staging: mt7621-mmc: Fix debug macro ERR_MSG and its usages
2018-08-22 9:13 ` Dan Carpenter
2018-08-22 11:16 ` Nishad Kamdar
@ 2018-08-23 17:15 ` Nishad Kamdar
1 sibling, 0 replies; 14+ messages in thread
From: Nishad Kamdar @ 2018-08-23 17:15 UTC (permalink / raw)
To: Dan Carpenter, Greg Kroah-Hartman
Cc: Joe Perches, NeilBrown, devel, Christian Lütke-Stetzkamp,
linux-kernel, John Crispin
On Wed, Aug 22, 2018 at 12:13:42PM +0300, Dan Carpenter wrote:
> On Wed, Aug 22, 2018 at 02:13:07PM +0530, Nishad Kamdar wrote:
> > diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
> > index 04d23cc7cd4a..6b2c72fc61f2 100644
> > --- a/drivers/staging/mt7621-mmc/sd.c
> > +++ b/drivers/staging/mt7621-mmc/sd.c
> > @@ -466,7 +466,8 @@ static void msdc_set_mclk(struct msdc_host *host, int ddr, unsigned int hz)
> > //u8 clksrc = hw->clk_src;
> >
> > if (!hz) { // set mmc system clock to 0 ?
> > - //ERR_MSG("set mclk to 0!!!");
> > + //dev_err(mmc_dev(host->mmc), "%d -> set mclk to 0!!!\n",
> > + // host->id);
>
> Just delete commented out code.
>
> > msdc_reset_hw(host);
> > return;
> > }
>
> regards,
> dan carpenter
>
Ok, I'll do that.
Thanks for the review.
regards,
nishad
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG
2018-08-22 11:38 ` Dan Carpenter
@ 2018-08-23 17:19 ` Nishad Kamdar
0 siblings, 0 replies; 14+ messages in thread
From: Nishad Kamdar @ 2018-08-23 17:19 UTC (permalink / raw)
To: Dan Carpenter, Greg Kroah-Hartman
Cc: Joe Perches, NeilBrown, devel, Christian Lütke-Stetzkamp,
linux-kernel, John Crispin
On Wed, Aug 22, 2018 at 02:38:44PM +0300, Dan Carpenter wrote:
> On Wed, Aug 22, 2018 at 04:40:56PM +0530, Nishad Kamdar wrote:
> > On Wed, Aug 22, 2018 at 12:09:36PM +0300, Dan Carpenter wrote:
> > > On Wed, Aug 22, 2018 at 02:04:55PM +0530, Nishad Kamdar wrote:
> > > > This patch fixes the debug macro N_MSG. Replaces printk with
> > > > dev_<level> without __func__ or __LINE__ or current->comm and
> > > > current->pid. Removes the do {} while(0) loop for the single
> > > > statement macro. Issue found by checkpatch.
> > > >
> > > > Signed-off-by: Nishad Kamdar <nishadkamdar@gmail.com>
> > > > ---
> > > > drivers/staging/mt7621-mmc/dbg.h | 11 ++++-------
> > > > 1 file changed, 4 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
> > > > index 2f2c56b73987..c56fb896617a 100644
> > > > --- a/drivers/staging/mt7621-mmc/dbg.h
> > > > +++ b/drivers/staging/mt7621-mmc/dbg.h
> > > > @@ -104,13 +104,10 @@ do { \
> > > >
> > > > #define N_MSG(evt, fmt, args...)
> > > > /*
> > > > -do { \
> > > > - if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > > > - printk(KERN_ERR TAG"%d -> "fmt" <- %s() : L<%d> PID<%s><0x%x>\n", \
> > > > - host->id, ##args , __FUNCTION__, __LINE__, current->comm, current->pid); \
> > > > - } \
> > > > -} while(0)
> > > > -*/
> > > > + *if ((DBG_EVT_##evt) & sd_debug_zone[host->id]) { \
> > > > + * dev_err(mmc_dev(host->mmc), "%d -> " fmt "\n", host->id, ##args) \
> > > > + *}
> > > > + */
> > >
> > > I don't understand what you're trying to do here. You just commented
> > > out the macro and turned it into a no-op. That's not what the patch
> > > description says.
> > >
> > > To me the original code seems fine.
> > >
> > > regards,
> > > dan carpenter
> > >
> >
> > Ok. But the macro definition was already commented.
> > I just changed printk to dev_err and gave it a better
> > block comment format.
> > Should I leave the older version as it is ?
>
> Oh... Yeah. You're right. Sorry about that.
>
> We normally just delete dead code without even thinking hard about it.
> It turns out that if we delete all the N_MSG() code, it's actually quite
> a bit. But probably it's the right thing to do, instead of hanging on
> to it like a hoarder.
>
> regards,
> dan carpenter
>
Ok. I get it.
Thanks for the review.
regards,
nishad
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-08-23 17:19 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-22 8:33 [PATCH v4 0/4] Fix debug macros and their usages Nishad Kamdar
2018-08-22 8:34 ` [PATCH v4 1/4] staging: mt7621-mmc: Fix debug macro N_MSG Nishad Kamdar
2018-08-22 9:09 ` Dan Carpenter
2018-08-22 11:10 ` Nishad Kamdar
2018-08-22 11:26 ` Greg Kroah-Hartman
2018-08-23 17:13 ` Nishad Kamdar
2018-08-22 11:38 ` Dan Carpenter
2018-08-23 17:19 ` Nishad Kamdar
2018-08-22 8:43 ` [PATCH v4 2/4] staging: mt7621-mmc: Fix debug macro ERR_MSG and its usages Nishad Kamdar
2018-08-22 9:13 ` Dan Carpenter
2018-08-22 11:16 ` Nishad Kamdar
2018-08-23 17:15 ` Nishad Kamdar
2018-08-22 8:45 ` [PATCH v4 3/4] staging: mt7621-mmc: Remove macro INIT_MSG " Nishad Kamdar
2018-08-22 8:47 ` [PATCH v4 4/4] staging: mt7621-mmc: Fix debug macro IRQ_MSG " Nishad Kamdar
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.