| Message ID | 20200820170951.v4.1.Ia54fe801f246a0b0aee36fb1f3bfb0922a8842b0@changeid (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,1/3,RESEND] media: camss: vfe: Use trace_printk for debugging only | expand |
On Thu, 20 Aug 2020 17:14:10 +0800 Nicolas Boichat <drinkcat@chromium.org> wrote: > trace_printk should not be used in production code. Since > tracing interrupts is presumably latency sensitive, pr_dbg is > not appropriate, so guard the call with a preprocessor symbol > that can be defined for debugging purpose. > > Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> > --- > > (resending this patch as part of the whole series, since we need a new > patch 3/3 now). > > drivers/media/platform/qcom/camss/camss-vfe-4-1.c | 2 ++ > drivers/media/platform/qcom/camss/camss-vfe-4-7.c | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c > index 174a36be6f5d866..0c57171fae4f9e9 100644 > --- a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c > +++ b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c > @@ -936,8 +936,10 @@ static irqreturn_t vfe_isr(int irq, void *dev) > > vfe->ops->isr_read(vfe, &value0, &value1); > > +#ifdef CAMSS_VFE_TRACE_IRQ > trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n", > value0, value1); Why are these not trace events? The reason I have that ugly banner is to keep people from doing EXACTLY THIS! trace_printk() is really easy to add, but it also is not configurable. When a trace_printk() is in the code, it is always enabled (well, you can turn trace_printk off, but that's an all or nothing. That is, by turning trace_printk off, you turn off *all* trace_printks). Instead, people should add trace events. This here is a perfect place to have a trace event. You don't need to add #ifdef around trace events because when not enabled, they are simply a nop. When enabled, the nop is turned into a jump to the tracing code. It should not affect performance. And as a trace event, you get a bunch of features with it (filtering, histograms, etc). -- Steve > +#endif > > if (value0 & VFE_0_IRQ_STATUS_0_RESET_ACK) > vfe->isr_ops.reset_ack(vfe); > diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c > index 0dca8bf9281e774..307675925e5c779 100644 > --- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c > +++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c > @@ -1058,8 +1058,10 @@ static irqreturn_t vfe_isr(int irq, void *dev) > > vfe->ops->isr_read(vfe, &value0, &value1); > > +#ifdef CAMSS_VFE_TRACE_IRQ > trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n", > value0, value1); > +#endif > > if (value0 & VFE_0_IRQ_STATUS_0_RESET_ACK) > vfe->isr_ops.reset_ack(vfe);
diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c index 174a36be6f5d866..0c57171fae4f9e9 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe-4-1.c +++ b/drivers/media/platform/qcom/camss/camss-vfe-4-1.c @@ -936,8 +936,10 @@ static irqreturn_t vfe_isr(int irq, void *dev) vfe->ops->isr_read(vfe, &value0, &value1); +#ifdef CAMSS_VFE_TRACE_IRQ trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n", value0, value1); +#endif if (value0 & VFE_0_IRQ_STATUS_0_RESET_ACK) vfe->isr_ops.reset_ack(vfe); diff --git a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c index 0dca8bf9281e774..307675925e5c779 100644 --- a/drivers/media/platform/qcom/camss/camss-vfe-4-7.c +++ b/drivers/media/platform/qcom/camss/camss-vfe-4-7.c @@ -1058,8 +1058,10 @@ static irqreturn_t vfe_isr(int irq, void *dev) vfe->ops->isr_read(vfe, &value0, &value1); +#ifdef CAMSS_VFE_TRACE_IRQ trace_printk("VFE: status0 = 0x%08x, status1 = 0x%08x\n", value0, value1); +#endif if (value0 & VFE_0_IRQ_STATUS_0_RESET_ACK) vfe->isr_ops.reset_ack(vfe);
trace_printk should not be used in production code. Since tracing interrupts is presumably latency sensitive, pr_dbg is not appropriate, so guard the call with a preprocessor symbol that can be defined for debugging purpose. Signed-off-by: Nicolas Boichat <drinkcat@chromium.org> --- (resending this patch as part of the whole series, since we need a new patch 3/3 now). drivers/media/platform/qcom/camss/camss-vfe-4-1.c | 2 ++ drivers/media/platform/qcom/camss/camss-vfe-4-7.c | 2 ++ 2 files changed, 4 insertions(+)