Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1410214 > unrolled thread
| Started by | Suzuki K Poulose <suzuki.poulose@arm.com> |
|---|---|
| First post | 2016-05-31 14:00 +0200 |
| Last post | 2016-06-01 17:20 +0200 |
| Articles | 5 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 4/5] coresight: Add better messages for coresight_timeout Suzuki K Poulose <suzuki.poulose@arm.com> - 2016-05-31 14:00 +0200
Re: [PATCH 4/5] coresight: Add better messages for coresight_timeout Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-05-31 20:00 +0200
Re: [PATCH 4/5] coresight: Add better messages for coresight_timeout Joe Perches <joe@perches.com> - 2016-05-31 20:00 +0200
Re: [PATCH 4/5] coresight: Add better messages for coresight_timeout Suzuki K Poulose <Suzuki.Poulose@arm.com> - 2016-06-01 11:40 +0200
Re: [PATCH 4/5] coresight: Add better messages for coresight_timeout Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-06-01 17:20 +0200
| From | Suzuki K Poulose <suzuki.poulose@arm.com> |
|---|---|
| Date | 2016-05-31 14:00 +0200 |
| Subject | [PATCH 4/5] coresight: Add better messages for coresight_timeout |
| Message-ID | <rEQv2-2eq-65@gated-at.bofh.it> |
When we encounter a timeout waiting for a status change via
coresight_timeout, the caller always print the offset which
was tried. This is pretty much useless as it doesn't specify
the bit position we wait for. Also, one needs to lookup the
TRM to figure out, what was wrong. This patch changes all
such error messages to print something more meaningful.
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
drivers/hwtracing/coresight/coresight-etb10.c | 6 ++----
drivers/hwtracing/coresight/coresight-etm4x.c | 6 ++----
drivers/hwtracing/coresight/coresight-tmc.c | 6 ++----
3 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
index 4d20b0b..6bd4b93 100644
--- a/drivers/hwtracing/coresight/coresight-etb10.c
+++ b/drivers/hwtracing/coresight/coresight-etb10.c
@@ -184,8 +184,7 @@ static void etb_disable_hw(struct etb_drvdata *drvdata)
if (coresight_timeout(drvdata->base, ETB_FFCR, ETB_FFCR_BIT, 0)) {
dev_err(drvdata->dev,
- "timeout observed when probing at offset %#x\n",
- ETB_FFCR);
+ "timeout while waiting for completion of Manual Flush\n");
}
/* disable trace capture */
@@ -193,8 +192,7 @@ static void etb_disable_hw(struct etb_drvdata *drvdata)
if (coresight_timeout(drvdata->base, ETB_FFSR, ETB_FFSR_BIT, 1)) {
dev_err(drvdata->dev,
- "timeout observed when probing at offset %#x\n",
- ETB_FFCR);
+ "timeout while waiting for Formatter to Stop\n");
}
CS_LOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 88947f3..e494042 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -111,8 +111,7 @@ static void etm4_enable_hw(void *info)
/* wait for TRCSTATR.IDLE to go up */
if (coresight_timeout(drvdata->base, TRCSTATR, TRCSTATR_IDLE_BIT, 1))
dev_err(drvdata->dev,
- "timeout observed when probing at offset %#x\n",
- TRCSTATR);
+ "timeout when waiting for Idle Trace Status\n");
writel_relaxed(config->pe_sel, drvdata->base + TRCPROCSELR);
writel_relaxed(config->cfg, drvdata->base + TRCCONFIGR);
@@ -184,8 +183,7 @@ static void etm4_enable_hw(void *info)
/* wait for TRCSTATR.IDLE to go back down to '0' */
if (coresight_timeout(drvdata->base, TRCSTATR, TRCSTATR_IDLE_BIT, 0))
dev_err(drvdata->dev,
- "timeout observed when probing at offset %#x\n",
- TRCSTATR);
+ "timeout when waiting for Idle Trace Status\n");
CS_LOCK(drvdata->base);
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
index 9e02ac9..c7d8ba6 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -38,8 +38,7 @@ void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata)
if (coresight_timeout(drvdata->base,
TMC_STS, TMC_STS_TMCREADY_BIT, 1)) {
dev_err(drvdata->dev,
- "timeout observed when probing at offset %#x\n",
- TMC_STS);
+ "timeout observed when waiting for TMC to be Ready\n");
}
}
@@ -56,8 +55,7 @@ void tmc_flush_and_stop(struct tmc_drvdata *drvdata)
if (coresight_timeout(drvdata->base,
TMC_FFCR, TMC_FFCR_FLUSHMAN_BIT, 0)) {
dev_err(drvdata->dev,
- "timeout observed when probing at offset %#x\n",
- TMC_FFCR);
+ "timeout observed while waiting for completion of Manual Flush\n");
}
tmc_wait_for_tmcready(drvdata);
--
1.9.1
[toc] | [next] | [standalone]
| From | Mathieu Poirier <mathieu.poirier@linaro.org> |
|---|---|
| Date | 2016-05-31 20:00 +0200 |
| Message-ID | <rEW7n-5MT-3@gated-at.bofh.it> |
| In reply to | #1410214 |
On 31 May 2016 at 05:57, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
> When we encounter a timeout waiting for a status change via
> coresight_timeout, the caller always print the offset which
> was tried. This is pretty much useless as it doesn't specify
> the bit position we wait for. Also, one needs to lookup the
> TRM to figure out, what was wrong. This patch changes all
> such error messages to print something more meaningful.
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> drivers/hwtracing/coresight/coresight-etb10.c | 6 ++----
> drivers/hwtracing/coresight/coresight-etm4x.c | 6 ++----
> drivers/hwtracing/coresight/coresight-tmc.c | 6 ++----
> 3 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index 4d20b0b..6bd4b93 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -184,8 +184,7 @@ static void etb_disable_hw(struct etb_drvdata *drvdata)
>
> if (coresight_timeout(drvdata->base, ETB_FFCR, ETB_FFCR_BIT, 0)) {
> dev_err(drvdata->dev,
> - "timeout observed when probing at offset %#x\n",
> - ETB_FFCR);
> + "timeout while waiting for completion of Manual Flush\n");
> }
>
> /* disable trace capture */
> @@ -193,8 +192,7 @@ static void etb_disable_hw(struct etb_drvdata *drvdata)
>
> if (coresight_timeout(drvdata->base, ETB_FFSR, ETB_FFSR_BIT, 1)) {
> dev_err(drvdata->dev,
> - "timeout observed when probing at offset %#x\n",
> - ETB_FFCR);
> + "timeout while waiting for Formatter to Stop\n");
> }
>
> CS_LOCK(drvdata->base);
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
> index 88947f3..e494042 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> @@ -111,8 +111,7 @@ static void etm4_enable_hw(void *info)
> /* wait for TRCSTATR.IDLE to go up */
> if (coresight_timeout(drvdata->base, TRCSTATR, TRCSTATR_IDLE_BIT, 1))
> dev_err(drvdata->dev,
> - "timeout observed when probing at offset %#x\n",
> - TRCSTATR);
> + "timeout when waiting for Idle Trace Status\n");
>
> writel_relaxed(config->pe_sel, drvdata->base + TRCPROCSELR);
> writel_relaxed(config->cfg, drvdata->base + TRCCONFIGR);
> @@ -184,8 +183,7 @@ static void etm4_enable_hw(void *info)
> /* wait for TRCSTATR.IDLE to go back down to '0' */
> if (coresight_timeout(drvdata->base, TRCSTATR, TRCSTATR_IDLE_BIT, 0))
> dev_err(drvdata->dev,
> - "timeout observed when probing at offset %#x\n",
> - TRCSTATR);
> + "timeout when waiting for Idle Trace Status\n");
>
> CS_LOCK(drvdata->base);
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
> index 9e02ac9..c7d8ba6 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
> @@ -38,8 +38,7 @@ void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata)
> if (coresight_timeout(drvdata->base,
> TMC_STS, TMC_STS_TMCREADY_BIT, 1)) {
> dev_err(drvdata->dev,
> - "timeout observed when probing at offset %#x\n",
> - TMC_STS);
> + "timeout observed when waiting for TMC to be Ready\n");
> }
> }
>
> @@ -56,8 +55,7 @@ void tmc_flush_and_stop(struct tmc_drvdata *drvdata)
> if (coresight_timeout(drvdata->base,
> TMC_FFCR, TMC_FFCR_FLUSHMAN_BIT, 0)) {
> dev_err(drvdata->dev,
> - "timeout observed when probing at offset %#x\n",
> - TMC_FFCR);
> + "timeout observed while waiting for completion of Manual Flush\n");
> }
>
> tmc_wait_for_tmcready(drvdata);
> --
> 1.9.1
>
I'm fine with this change but in all 3 files the beginning of the new
error message is different. Please select one and use it throughout.
Thanks,
Mathieu
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2016-05-31 20:00 +0200 |
| Message-ID | <rEW7n-5MT-21@gated-at.bofh.it> |
| In reply to | #1410214 |
On Tue, 2016-05-31 at 12:57 +0100, Suzuki K Poulose wrote:
> When we encounter a timeout waiting for a status change via
> coresight_timeout, the caller always print the offset which
> was tried. This is pretty much useless as it doesn't specify
> the bit position we wait for. Also, one needs to lookup the
> TRM to figure out, what was wrong. This patch changes all
> such error messages to print something more meaningful.
trivia:
Perhaps consistently using
dev_err(dev, "timeout while waiting for %s\n", "<foo>");
could make the object code a bit smaller.
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
[]
> @@ -184,8 +184,7 @@ static void etb_disable_hw(struct etb_drvdata *drvdata)
>
> if (coresight_timeout(drvdata->base, ETB_FFCR, ETB_FFCR_BIT, 0)) {
> dev_err(drvdata->dev,
> - "timeout observed when probing at offset %#x\n",
> - ETB_FFCR);
> + "timeout while waiting for completion of Manual Flush\n");
ie:
dev_err(drvdata->dev,
"timeout while waiting for %s\n",
"completion of Manual Flush");
but that depends on how many of these coresight
files are compiled and linked.
There is a while/when usage difference in some of
the output messages.
[toc] | [prev] | [next] | [standalone]
| From | Suzuki K Poulose <Suzuki.Poulose@arm.com> |
|---|---|
| Date | 2016-06-01 11:40 +0200 |
| Message-ID | <rFaN4-6FG-19@gated-at.bofh.it> |
| In reply to | #1410503 |
On 31/05/16 18:58, Joe Perches wrote:
> On Tue, 2016-05-31 at 12:57 +0100, Suzuki K Poulose wrote:
>> When we encounter a timeout waiting for a status change via
>> coresight_timeout, the caller always print the offset which
>> was tried. This is pretty much useless as it doesn't specify
>> the bit position we wait for. Also, one needs to lookup the
>> TRM to figure out, what was wrong. This patch changes all
>> such error messages to print something more meaningful.
>
> trivia:
>
> Perhaps consistently using
> dev_err(dev, "timeout while waiting for %s\n", "<foo>");
> could make the object code a bit smaller.
>
>> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> []
>> @@ -184,8 +184,7 @@ static void etb_disable_hw(struct etb_drvdata *drvdata)
>>
>> if (coresight_timeout(drvdata->base, ETB_FFCR, ETB_FFCR_BIT, 0)) {
>> dev_err(drvdata->dev,
>> - "timeout observed when probing at offset %#x\n",
>> - ETB_FFCR);
>> + "timeout while waiting for completion of Manual Flush\n");
>
> ie:
> dev_err(drvdata->dev,
> "timeout while waiting for %s\n",
> "completion of Manual Flush");
>
> but that depends on how many of these coresight
> files are compiled and linked.
Or we could move the timeout message to coresight_timeout(). The only disadvantage is
if a caller is OK with silent timeouts. How about :
int coresight_timeout(void *base, u32 offset, u32 bit, u32 val, char *info)
where the message can be suppressed if info == NULL ?
Mathieu, your thoughts ?
>
> There is a while/when usage difference in some of
> the output messages.
Right, I will fix them. This was a merged version of individual patches, hence
the changes.
Cheers
Suzuki
[toc] | [prev] | [next] | [standalone]
| From | Mathieu Poirier <mathieu.poirier@linaro.org> |
|---|---|
| Date | 2016-06-01 17:20 +0200 |
| Message-ID | <rFg65-1A8-17@gated-at.bofh.it> |
| In reply to | #1411030 |
On 1 June 2016 at 03:34, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 31/05/16 18:58, Joe Perches wrote:
>>
>> On Tue, 2016-05-31 at 12:57 +0100, Suzuki K Poulose wrote:
>>>
>>> When we encounter a timeout waiting for a status change via
>>> coresight_timeout, the caller always print the offset which
>>> was tried. This is pretty much useless as it doesn't specify
>>> the bit position we wait for. Also, one needs to lookup the
>>> TRM to figure out, what was wrong. This patch changes all
>>> such error messages to print something more meaningful.
>>
>>
>> trivia:
>>
>> Perhaps consistently using
>> dev_err(dev, "timeout while waiting for %s\n", "<foo>");
>> could make the object code a bit smaller.
>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c
>>> b/drivers/hwtracing/coresight/coresight-etb10.c
>>
>> []
>>>
>>> @@ -184,8 +184,7 @@ static void etb_disable_hw(struct etb_drvdata
>>> *drvdata)
>>>
>>> if (coresight_timeout(drvdata->base, ETB_FFCR, ETB_FFCR_BIT, 0))
>>> {
>>> dev_err(drvdata->dev,
>>> - "timeout observed when probing at offset %#x\n",
>>> - ETB_FFCR);
>>> + "timeout while waiting for completion of Manual
>>> Flush\n");
>>
>>
>> ie:
>> dev_err(drvdata->dev,
>> "timeout while waiting for %s\n",
>> "completion of Manual Flush");
>>
>> but that depends on how many of these coresight
>> files are compiled and linked.
>
>
> Or we could move the timeout message to coresight_timeout(). The only
> disadvantage is
> if a caller is OK with silent timeouts. How about :
>
> int coresight_timeout(void *base, u32 offset, u32 bit, u32 val, char *info)
>
> where the message can be suppressed if info == NULL ?
>
> Mathieu, your thoughts ?
I'd rather keep things separate.
Thanks,
Mathieu
>
>>
>> There is a while/when usage difference in some of
>> the output messages.
>
>
> Right, I will fix them. This was a merged version of individual patches,
> hence
> the changes.
>
> Cheers
> Suzuki
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web