public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jaehoon Chung <jh80.chung@samsung.com>
To: Jisheng Zhang <jszhang@marvell.com>,
	Baolin Wang <baolin.wang@linaro.org>
Cc: ulf.hansson@linaro.org, adrian.hunter@intel.com,
	rmk+kernel@arm.linux.org.uk, shawn.lin@rock-chips.com,
	dianders@chromium.org, david@protonic.nl, broonie@kernel.org,
	linus.walleij@linaro.org, takahiro.akashi@linaro.org,
	linux-kernel@vger.kernel.org, linux-mmc@vger.kernel.org
Subject: Re: [PATCH] mmc: Remove some redundant debug prints
Date: Thu, 07 Apr 2016 10:40:42 +0900	[thread overview]
Message-ID: <5705BA9A.6050300@samsung.com> (raw)
In-Reply-To: <20160406195732.67430c96@xhacker>

On 04/06/2016 08:57 PM, Jisheng Zhang wrote:
> 
> 
> On Wed,  6 Apr 2016 19:38:30 +0800 Baolin Wang wrote:
> 
>> This patch removes some redundant debug prints, since we have added some
>> tracepoints to help with performance analysis of MMC subsystem.
> 
> I think the debug prints you removed are useful for debugging mmc err, how is
> this purpose achieved by tracepoints? From another side, why should I enable
> tracepoints to debug mmc err?

I agreed Jisheng's opinion. tracepoint is helpful for analyzing performance and debugging something.
But I think it's more easier to check the mmc error at booting time or some time.

Best Regards,
Jaehoon Chung

> 
> Thanks
> 
>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>> ---
>>  drivers/mmc/core/core.c |   52 -----------------------------------------------
>>  1 file changed, 52 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index f80b3ab..3f1362a 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -157,33 +157,6 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq)
>>  
>>  		led_trigger_event(host->led, LED_OFF);
>>  
>> -		if (mrq->sbc) {
>> -			pr_debug("%s: req done <CMD%u>: %d: %08x %08x %08x %08x\n",
>> -				mmc_hostname(host), mrq->sbc->opcode,
>> -				mrq->sbc->error,
>> -				mrq->sbc->resp[0], mrq->sbc->resp[1],
>> -				mrq->sbc->resp[2], mrq->sbc->resp[3]);
>> -		}
>> -
>> -		pr_debug("%s: req done (CMD%u): %d: %08x %08x %08x %08x\n",
>> -			mmc_hostname(host), cmd->opcode, err,
>> -			cmd->resp[0], cmd->resp[1],
>> -			cmd->resp[2], cmd->resp[3]);
>> -
>> -		if (mrq->data) {
>> -			pr_debug("%s:     %d bytes transferred: %d\n",
>> -				mmc_hostname(host),
>> -				mrq->data->bytes_xfered, mrq->data->error);
>> -		}
>> -
>> -		if (mrq->stop) {
>> -			pr_debug("%s:     (CMD%u): %d: %08x %08x %08x %08x\n",
>> -				mmc_hostname(host), mrq->stop->opcode,
>> -				mrq->stop->error,
>> -				mrq->stop->resp[0], mrq->stop->resp[1],
>> -				mrq->stop->resp[2], mrq->stop->resp[3]);
>> -		}
>> -
>>  		if (mrq->done)
>>  			mrq->done(mrq);
>>  	}
>> @@ -236,31 +209,6 @@ static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
>>  	if (mmc_card_removed(host->card))
>>  		return -ENOMEDIUM;
>>  
>> -	if (mrq->sbc) {
>> -		pr_debug("<%s: starting CMD%u arg %08x flags %08x>\n",
>> -			 mmc_hostname(host), mrq->sbc->opcode,
>> -			 mrq->sbc->arg, mrq->sbc->flags);
>> -	}
>> -
>> -	pr_debug("%s: starting CMD%u arg %08x flags %08x\n",
>> -		 mmc_hostname(host), mrq->cmd->opcode,
>> -		 mrq->cmd->arg, mrq->cmd->flags);
>> -
>> -	if (mrq->data) {
>> -		pr_debug("%s:     blksz %d blocks %d flags %08x "
>> -			"tsac %d ms nsac %d\n",
>> -			mmc_hostname(host), mrq->data->blksz,
>> -			mrq->data->blocks, mrq->data->flags,
>> -			mrq->data->timeout_ns / 1000000,
>> -			mrq->data->timeout_clks);
>> -	}
>> -
>> -	if (mrq->stop) {
>> -		pr_debug("%s:     CMD%u arg %08x flags %08x\n",
>> -			 mmc_hostname(host), mrq->stop->opcode,
>> -			 mrq->stop->arg, mrq->stop->flags);
>> -	}
>> -
>>  	WARN_ON(!host->claimed);
>>  
>>  	mrq->cmd->error = 0;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

  reply	other threads:[~2016-04-07  1:40 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-06 11:38 [PATCH] mmc: Remove some redundant debug prints Baolin Wang
2016-04-06 11:57 ` Jisheng Zhang
2016-04-07  1:40   ` Jaehoon Chung [this message]
2016-04-07  3:15     ` Baolin Wang
2016-04-07  3:13   ` Baolin Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5705BA9A.6050300@samsung.com \
    --to=jh80.chung@samsung.com \
    --cc=adrian.hunter@intel.com \
    --cc=baolin.wang@linaro.org \
    --cc=broonie@kernel.org \
    --cc=david@protonic.nl \
    --cc=dianders@chromium.org \
    --cc=jszhang@marvell.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=rmk+kernel@arm.linux.org.uk \
    --cc=shawn.lin@rock-chips.com \
    --cc=takahiro.akashi@linaro.org \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox