Re: [PATCH] drm/msm/dpu: make error messages at dpu_core_irq_register_callback() more sensible
On Sat, 30 Mar 2024 05:53:22 +0200, Dmitry Baryshkov wrote: > There is little point in using %ps to print a value known to be NULL. On > the other hand it makes sense to print the callback symbol in the > 'invalid IRQ' message. Correct those two error messages to make more > sense. > > Applied, thanks! [1/1] drm/msm/dpu: make error messages at dpu_core_irq_register_callback() more sensible https://gitlab.freedesktop.org/drm/msm/-/commit/8844f467d6a5 Best regards, -- Abhinav Kumar
Re: [PATCH] drm/msm/dpu: make error messages at dpu_core_irq_register_callback() more sensible
On 2024-03-30 11:31:37, Marijn Suijten wrote:
> On 2024-03-30 05:53:22, Dmitry Baryshkov wrote:
> > There is little point in using %ps to print a value known to be NULL. On
> > the other hand it makes sense to print the callback symbol in the
> > 'invalid IRQ' message. Correct those two error messages to make more
> > sense.
> >
> > Fixes: 6893199183f8 ("drm/msm/dpu: stop using raw IRQ indices in the kernel
> > output")
> > Signed-off-by: Dmitry Baryshkov
>
> Agreed, this is a lot more clear:
>
> Reviewed-by: Marijn Suijten
Note that, as seen in [1], there are still a few codepaths that only print
"invalid IRQ" without any additional context (if we could even add it - __func__
might be a good start) and inconsistently use DPU_ERROR vs pr_err too :/
Any possibility to address that?
- Marijn
[1]:
https://lore.kernel.org/linux-arm-msm/mxwrvnqth5f2vd4m55ryzqgyj7brykiqynzldelanxkuj2zny3@4pqi6p57c2q2/
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 8
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> > index 946dd0135dff..6a0a74832fb6 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> > @@ -525,14 +525,14 @@ int dpu_core_irq_register_callback(struct dpu_kms
> > *dpu_kms,
> > int ret;
> >
> > if (!irq_cb) {
> > - DPU_ERROR("invalid IRQ=[%d, %d] irq_cb:%ps\n",
> > - DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx), irq_cb);
> > + DPU_ERROR("IRQ=[%d, %d] NULL callback\n",
> > + DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx));
> > return -EINVAL;
> > }
> >
> > if (!dpu_core_irq_is_valid(irq_idx)) {
> > - DPU_ERROR("invalid IRQ=[%d, %d]\n",
> > - DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx));
> > + DPU_ERROR("invalid IRQ=[%d, %d] irq_cb:%ps\n",
> > + DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx), irq_cb);
> > return -EINVAL;
> > }
> >
> >
> > ---
> > base-commit: 13ee4a7161b6fd938aef6688ff43b163f6d83e37
> > change-id: 20240330-dpu-irq-messages-5cf13fd7568c
> >
> > Best regards,
> > --
> > Dmitry Baryshkov
> >
Re: [PATCH] drm/msm/dpu: make error messages at dpu_core_irq_register_callback() more sensible
On 3/29/2024 8:53 PM, Dmitry Baryshkov wrote:
There is little point in using %ps to print a value known to be NULL. On
the other hand it makes sense to print the callback symbol in the
'invalid IRQ' message. Correct those two error messages to make more
sense.
Fixes: 6893199183f8 ("drm/msm/dpu: stop using raw IRQ indices in the kernel
output")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Abhinav Kumar
Re: [PATCH] drm/msm/dpu: make error messages at dpu_core_irq_register_callback() more sensible
On 2024-03-30 05:53:22, Dmitry Baryshkov wrote:
> There is little point in using %ps to print a value known to be NULL. On
> the other hand it makes sense to print the callback symbol in the
> 'invalid IRQ' message. Correct those two error messages to make more
> sense.
>
> Fixes: 6893199183f8 ("drm/msm/dpu: stop using raw IRQ indices in the kernel
> output")
> Signed-off-by: Dmitry Baryshkov
Agreed, this is a lot more clear:
Reviewed-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> index 946dd0135dff..6a0a74832fb6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
> @@ -525,14 +525,14 @@ int dpu_core_irq_register_callback(struct dpu_kms
> *dpu_kms,
> int ret;
>
> if (!irq_cb) {
> - DPU_ERROR("invalid IRQ=[%d, %d] irq_cb:%ps\n",
> - DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx), irq_cb);
> + DPU_ERROR("IRQ=[%d, %d] NULL callback\n",
> + DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx));
> return -EINVAL;
> }
>
> if (!dpu_core_irq_is_valid(irq_idx)) {
> - DPU_ERROR("invalid IRQ=[%d, %d]\n",
> - DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx));
> + DPU_ERROR("invalid IRQ=[%d, %d] irq_cb:%ps\n",
> + DPU_IRQ_REG(irq_idx), DPU_IRQ_BIT(irq_idx), irq_cb);
> return -EINVAL;
> }
>
>
> ---
> base-commit: 13ee4a7161b6fd938aef6688ff43b163f6d83e37
> change-id: 20240330-dpu-irq-messages-5cf13fd7568c
>
> Best regards,
> --
> Dmitry Baryshkov
>
