[PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
IOMMU upper layer is already printing error message. OMAP2+ specific
layer may print error message only for debug purpose.
Signed-off-by: David Cohen daco...@gmail.com
---
arch/arm/mach-omap2/iommu2.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 14ee686..4244a07 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32
*ra)
da = iommu_read_reg(obj, MMU_FAULT_AD);
*ra = da;
- dev_err(obj-dev, %s:\tda:%08x , __func__, da);
+ dev_dbg(obj-dev, %s:\tda:%08x , __func__, da);
for (i = 0; i ARRAY_SIZE(err_msg); i++) {
if (stat (1 i))
- printk(%s , err_msg[i]);
+ printk(KERN_DEBUG %s , err_msg[i]);
}
- printk(\n);
+ printk(KERN_DEBUG \n);
iommu_write_reg(obj, stat, MMU_IRQSTATUS);
--
1.7.2.3
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
Hello.
On 15-02-2011 16:20, David Cohen wrote:
IOMMU upper layer is already printing error message. OMAP2+ specific
layer may print error message only for debug purpose.
Signed-off-by: David Cohendaco...@gmail.com
---
arch/arm/mach-omap2/iommu2.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 14ee686..4244a07 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32
*ra)
da = iommu_read_reg(obj, MMU_FAULT_AD);
*ra = da;
- dev_err(obj-dev, %s:\tda:%08x , __func__, da);
+ dev_dbg(obj-dev, %s:\tda:%08x , __func__, da);
Note that dev_dbg() will only print something if either DEBUG or
CONFIG_DYNAMIC_DEBUG are defined...
for (i = 0; i ARRAY_SIZE(err_msg); i++) {
if (stat (1 i))
- printk(%s , err_msg[i]);
+ printk(KERN_DEBUG %s , err_msg[i]);
... unlike printk(KERN_DEBUG...). You probably want to use pr_debug()
instead.
}
- printk(\n);
+ printk(KERN_DEBUG \n);
Here too... Although wait, it should be KERN_CONT instead! Debug levels
are only attributed to the whole lines.
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
On Tue, Feb 15, 2011 at 3:38 PM, Sergei Shtylyov sshtyl...@mvista.com wrote:
Hello.
Hi,
On 15-02-2011 16:20, David Cohen wrote:
IOMMU upper layer is already printing error message. OMAP2+ specific
layer may print error message only for debug purpose.
Signed-off-by: David Cohendaco...@gmail.com
---
arch/arm/mach-omap2/iommu2.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 14ee686..4244a07 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj,
u32 *ra)
da = iommu_read_reg(obj, MMU_FAULT_AD);
*ra = da;
- dev_err(obj-dev, %s:\tda:%08x , __func__, da);
+ dev_dbg(obj-dev, %s:\tda:%08x , __func__, da);
Note that dev_dbg() will only print something if either DEBUG or
CONFIG_DYNAMIC_DEBUG are defined...
That's my plan.
for (i = 0; i ARRAY_SIZE(err_msg); i++) {
if (stat (1 i))
- printk(%s , err_msg[i]);
+ printk(KERN_DEBUG %s , err_msg[i]);
... unlike printk(KERN_DEBUG...). You probably want to use pr_debug()
instead.
}
- printk(\n);
+ printk(KERN_DEBUG \n);
Here too... Although wait, it should be KERN_CONT instead! Debug levels
are only attributed to the whole lines.
But your observation is correct. I'll resend it with KERN_CONT instead.
Regards,
David
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
On 15-02-2011 16:44, David Cohen wrote:
IOMMU upper layer is already printing error message. OMAP2+ specific
layer may print error message only for debug purpose.
Signed-off-by: David Cohendaco...@gmail.com
---
arch/arm/mach-omap2/iommu2.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 14ee686..4244a07 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj,
u32 *ra)
da = iommu_read_reg(obj, MMU_FAULT_AD);
*ra = da;
- dev_err(obj-dev, %s:\tda:%08x , __func__, da);
+ dev_dbg(obj-dev, %s:\tda:%08x , __func__, da);
Note that dev_dbg() will only print something if either DEBUG or
CONFIG_DYNAMIC_DEBUG are defined...
That's my plan.
for (i = 0; i ARRAY_SIZE(err_msg); i++) {
if (stat (1i))
- printk(%s , err_msg[i]);
+ printk(KERN_DEBUG %s , err_msg[i]);
... unlike printk(KERN_DEBUG...). You probably want to use pr_debug()
instead.
}
- printk(\n);
+ printk(KERN_DEBUG \n);
Here too... Although wait, it should be KERN_CONT instead! Debug levels
are only attributed to the whole lines.
But your observation is correct. I'll resend it with KERN_CONT instead.
This won't play out correctly anyway. If DEBUG is not #define'd, the
beginning of line won't be printed but the continuations will. You just can't
do it correctly with dev_dbg(), unless you break the single line into several
ones.
Regards,
David
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
On Tue, 15 Feb 2011 15:44:27 +0200 David Cohen daco...@gmail.com wrote: @@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) da = iommu_read_reg(obj, MMU_FAULT_AD); *ra = da; - dev_err(obj-dev, %s:\tda:%08x , __func__, da); + dev_dbg(obj-dev, %s:\tda:%08x , __func__, da); Note that dev_dbg() will only print something if either DEBUG or CONFIG_DYNAMIC_DEBUG are defined... That's my plan. So it's sure that a developer won't need these error dumps when receiving an error report? I.e. IOMMU upper level errors give enough information to start doing own debugging? Just my 2 cents. -- Jarkko -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
On Tue, Feb 15, 2011 at 3:56 PM, Sergei Shtylyov sshtyl...@mvista.com wrote:
On 15-02-2011 16:44, David Cohen wrote:
IOMMU upper layer is already printing error message. OMAP2+ specific
layer may print error message only for debug purpose.
Signed-off-by: David Cohendaco...@gmail.com
---
arch/arm/mach-omap2/iommu2.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 14ee686..4244a07 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu
*obj,
u32 *ra)
da = iommu_read_reg(obj, MMU_FAULT_AD);
*ra = da;
- dev_err(obj-dev, %s:\tda:%08x , __func__, da);
+ dev_dbg(obj-dev, %s:\tda:%08x , __func__, da);
Note that dev_dbg() will only print something if either DEBUG or
CONFIG_DYNAMIC_DEBUG are defined...
That's my plan.
for (i = 0; i ARRAY_SIZE(err_msg); i++) {
if (stat (1 i))
- printk(%s , err_msg[i]);
+ printk(KERN_DEBUG %s , err_msg[i]);
... unlike printk(KERN_DEBUG...). You probably want to use pr_debug()
instead.
}
- printk(\n);
+ printk(KERN_DEBUG \n);
Here too... Although wait, it should be KERN_CONT instead! Debug levels
are only attributed to the whole lines.
But your observation is correct. I'll resend it with KERN_CONT instead.
This won't play out correctly anyway. If DEBUG is not #define'd, the
beginning of line won't be printed but the continuations will. You just
can't do it correctly with dev_dbg(), unless you break the single line into
several ones.
Yes, I got this situation. I'm coming with a proper solution on next version.
Br,
David
Regards,
David
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
On Tue, Feb 15, 2011 at 3:59 PM, Jarkko Nikula jhnik...@gmail.com wrote: On Tue, 15 Feb 2011 15:44:27 +0200 David Cohen daco...@gmail.com wrote: @@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) da = iommu_read_reg(obj, MMU_FAULT_AD); *ra = da; - dev_err(obj-dev, %s:\tda:%08x , __func__, da); + dev_dbg(obj-dev, %s:\tda:%08x , __func__, da); Note that dev_dbg() will only print something if either DEBUG or CONFIG_DYNAMIC_DEBUG are defined... That's my plan. So it's sure that a developer won't need these error dumps when receiving an error report? I.e. IOMMU upper level errors give enough information to start doing own debugging? Yes, developers do need this information. But it's a bit useless tell only we've got an iommu fault, due to many places might be causing it. My purpose is to let the debug responsibility to IOMMU users. They have access to the iovmm layer as well and can provide a much more useful information. e.g. OMAP3 ISP has many submodules using IOMMU. With a fault callback, it can dump all the iovm areas and the faulty 'da' too. It might indicate which submodule was responsible for the issue. Of course we can just let this debug messages the way they are and print this redundant information. But IMO it's not necessary. Regards, David Just my 2 cents. -- Jarkko -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
On Tue, Feb 15, 2011 at 04:38:32PM +0300, Sergei Shtylyov wrote:
diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 14ee686..4244a07 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj,
u32 *ra)
da = iommu_read_reg(obj, MMU_FAULT_AD);
*ra = da;
-dev_err(obj-dev, %s:\tda:%08x , __func__, da);
+dev_dbg(obj-dev, %s:\tda:%08x , __func__, da);
Note that dev_dbg() will only print something if either DEBUG or
CONFIG_DYNAMIC_DEBUG are defined...
for (i = 0; i ARRAY_SIZE(err_msg); i++) {
if (stat (1 i))
-printk(%s , err_msg[i]);
+printk(KERN_DEBUG %s , err_msg[i]);
... unlike printk(KERN_DEBUG...). You probably want to use pr_debug()
instead.
No - this isn't starting a new line. pr_cont() here.
}
-printk(\n);
+printk(KERN_DEBUG \n);
Here too... Although wait, it should be KERN_CONT instead! Debug
levels are only attributed to the whole lines.
And pr_cont() here too. If you care about using KERN_CONT which is
just a static marker to allow automated printk level checking easier.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
On Tue, 15 Feb 2011 16:08:32 +0200 David Cohen daco...@gmail.com wrote: So it's sure that a developer won't need these error dumps when receiving an error report? I.e. IOMMU upper level errors give enough information to start doing own debugging? Yes, developers do need this information. But it's a bit useless tell only we've got an iommu fault, due to many places might be causing it. My purpose is to let the debug responsibility to IOMMU users. They have access to the iovmm layer as well and can provide a much more useful information. e.g. OMAP3 ISP has many submodules using IOMMU. With a fault callback, it can dump all the iovm areas and the faulty 'da' too. It might indicate which submodule was responsible for the issue. Of course we can just let this debug messages the way they are and print this redundant information. But IMO it's not necessary. Sounds fair enough and if I understood correctly this is not something what end user will hit but more like another developer. In that case the debug messages are the right thing. -- Jarkko -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
On Tue, Feb 15, 2011 at 4:29 PM, Russell King - ARM Linux
li...@arm.linux.org.uk wrote:
On Tue, Feb 15, 2011 at 04:38:32PM +0300, Sergei Shtylyov wrote:
diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
index 14ee686..4244a07 100644
--- a/arch/arm/mach-omap2/iommu2.c
+++ b/arch/arm/mach-omap2/iommu2.c
@@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj,
u32 *ra)
da = iommu_read_reg(obj, MMU_FAULT_AD);
*ra = da;
- dev_err(obj-dev, %s:\tda:%08x , __func__, da);
+ dev_dbg(obj-dev, %s:\tda:%08x , __func__, da);
Note that dev_dbg() will only print something if either DEBUG or
CONFIG_DYNAMIC_DEBUG are defined...
for (i = 0; i ARRAY_SIZE(err_msg); i++) {
if (stat (1 i))
- printk(%s , err_msg[i]);
+ printk(KERN_DEBUG %s , err_msg[i]);
... unlike printk(KERN_DEBUG...). You probably want to use pr_debug()
instead.
No - this isn't starting a new line. pr_cont() here.
But pr_cont() would be wrong in case of DEBUG isn't set, isn't it?
}
- printk(\n);
+ printk(KERN_DEBUG \n);
Here too... Although wait, it should be KERN_CONT instead! Debug
levels are only attributed to the whole lines.
And pr_cont() here too. If you care about using KERN_CONT which is
just a static marker to allow automated printk level checking easier.
The same situation here.
But this patch was dropped in the next version.
Br,
David
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
On Tue, Feb 15, 2011 at 4:08 PM, David Cohen daco...@gmail.com wrote: On Tue, Feb 15, 2011 at 3:59 PM, Jarkko Nikula jhnik...@gmail.com wrote: On Tue, 15 Feb 2011 15:44:27 +0200 David Cohen daco...@gmail.com wrote: @@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra) da = iommu_read_reg(obj, MMU_FAULT_AD); *ra = da; - dev_err(obj-dev, %s:\tda:%08x , __func__, da); + dev_dbg(obj-dev, %s:\tda:%08x , __func__, da); Note that dev_dbg() will only print something if either DEBUG or CONFIG_DYNAMIC_DEBUG are defined... That's my plan. So it's sure that a developer won't need these error dumps when receiving an error report? I.e. IOMMU upper level errors give enough information to start doing own debugging? Yes, developers do need this information. But it's a bit useless tell only we've got an iommu fault, due to many places might be causing it. My purpose is to let the debug responsibility to IOMMU users. They have access to the iovmm layer as well and can provide a much more useful information. e.g. OMAP3 ISP has many submodules using IOMMU. With a fault callback, it can dump all the iovm areas and the faulty 'da' too. It might indicate which submodule was responsible for the issue. Of course we can just let this debug messages the way they are and print this redundant information. But IMO it's not necessary. Indeed, we can leave this discussion for future. My main purpose now is the fault callback. I'll drop this patch 1/2 for now. Regards, David Regards, David Just my 2 cents. -- Jarkko -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
On Tue, Feb 15, 2011 at 04:36:26PM +0200, David Cohen wrote: But pr_cont() would be wrong in case of DEBUG isn't set, isn't it? Yes. One other solution you could do is: char buf[80], *p = buf; buf[0] = '\0'; for (i = 0; i ARRAY_SIZE(err_msg); i++) if (stat (1 i)) p += scnprintf(p, sizeof(buf) - (p - buf), %s, err_msg[i]); dev_dbg(obj-dev, %s: da:%08x%s\n, __func__, da, buf); which means that you're then printing the entire string in one go - and there's no chance for another message to come in the middle of it. Note that I've placed the ' ' at the beginning of the format string so that we don't end up with useless trailing space in messages. Minor point but it's easy to do here. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
On Tue, Feb 15, 2011 at 4:44 PM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Feb 15, 2011 at 04:36:26PM +0200, David Cohen wrote: But pr_cont() would be wrong in case of DEBUG isn't set, isn't it? Yes. One other solution you could do is: char buf[80], *p = buf; buf[0] = '\0'; for (i = 0; i ARRAY_SIZE(err_msg); i++) if (stat (1 i)) p += scnprintf(p, sizeof(buf) - (p - buf), %s, err_msg[i]); dev_dbg(obj-dev, %s: da:%08x%s\n, __func__, da, buf); which means that you're then printing the entire string in one go - and there's no chance for another message to come in the middle of it. Note that I've placed the ' ' at the beginning of the format string so that we don't end up with useless trailing space in messages. Minor point but it's easy to do here. That could be my choice. I'm not planing to resend this patch, but how good/bad it sounds to you to have dev_dbg_cont() for such situation? Br, David -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
On Tue, Feb 15, 2011 at 04:50:29PM +0200, David Cohen wrote: That could be my choice. I'm not planing to resend this patch, but how good/bad it sounds to you to have dev_dbg_cont() for such situation? That doesn't help when the message gets corrupted by another thread, so I'd much prefer the format line then print approach rather than the blah_CONT stuff. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
