On Thu, 20 Aug 2020 17:14:10 +0800
Nicolas Boichat <drinkcat@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> ---
>
> (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);
