linux-kernel - Re: [PATCH] drm/msm/dpu: Print interrupt index in addition to the mask

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 21 Nov 2022 23:35:46 +0100
From:   Martin Botka <martin.botka@...ainline.org>
To:     Marijn Suijten <marijn.suijten@...ainline.org>,
        phone-devel@...r.kernel.org
CC:     ~postmarketos/upstreaming@...ts.sr.ht,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Jami Kettunen <jami.kettunen@...ainline.org>,
        Rob Clark <robdclark@...il.com>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Sean Paul <sean@...rly.run>, David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Stephen Boyd <swboyd@...omium.org>,
        Vinod Koul <vkoul@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Vinod Polimera <quic_vpolimer@...cinc.com>,
        linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/msm/dpu: Print interrupt index in addition to the mask



On November 21, 2022 11:24:55 PM GMT+01:00, Marijn Suijten <marijn.suijten@...ainline.org> wrote:
>The mask only describes the `irq_idx % 32` part, making it generally
>impossible to deduce what interrupt is being enabled/disabled.  Since
>`debug/core_irq` in debugfs (and other prints) also include the full
>`DPU_IRQ_IDX()` value, print the same full value here for easier
>correlation instead of only adding the `irq_idx / 32` part.
>
>Furthermore, make the dbgstr messages more consistent.
>
>Signed-off-by: Marijn Suijten <marijn.suijten@...ainline.org>
>---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 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 cf1b6d84c18a..64589a9c2c51 100644
>--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
>+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c
>@@ -252,9 +252,9 @@ static int dpu_hw_intr_enable_irq_locked(struct dpu_hw_intr *intr, int irq_idx)
> 
> 	cache_irq_mask = intr->cache_irq_mask[reg_idx];
> 	if (cache_irq_mask & DPU_IRQ_MASK(irq_idx)) {
>-		dbgstr = "DPU IRQ already set:";
>+		dbgstr = "already ";
> 	} else {
>-		dbgstr = "DPU IRQ enabled:";
>+		dbgstr = "";
> 
> 		cache_irq_mask |= DPU_IRQ_MASK(irq_idx);
> 		/* Cleaning any pending interrupt */
>@@ -268,7 +268,7 @@ static int dpu_hw_intr_enable_irq_locked(struct dpu_hw_intr *intr, int irq_idx)
> 		intr->cache_irq_mask[reg_idx] = cache_irq_mask;
> 	}
> 
>-	pr_debug("%s MASK:0x%.8lx, CACHE-MASK:0x%.8x\n", dbgstr,
>+	pr_debug("DPU IRQ %d %senabled: MASK:0x%.8lx, CACHE-MASK:0x%.8x\n", irq_idx, dbgstr,
> 			DPU_IRQ_MASK(irq_idx), cache_irq_mask);
> 
> 	return 0;
>@@ -301,9 +301,9 @@ static int dpu_hw_intr_disable_irq_locked(struct dpu_hw_intr *intr, int irq_idx)
> 
> 	cache_irq_mask = intr->cache_irq_mask[reg_idx];
> 	if ((cache_irq_mask & DPU_IRQ_MASK(irq_idx)) == 0) {
>-		dbgstr = "DPU IRQ is already cleared:";
>+		dbgstr = "already ";
> 	} else {
>-		dbgstr = "DPU IRQ mask disable:";
>+		dbgstr = "";
> 
> 		cache_irq_mask &= ~DPU_IRQ_MASK(irq_idx);
> 		/* Disable interrupts based on the new mask */
>@@ -317,7 +317,7 @@ static int dpu_hw_intr_disable_irq_locked(struct dpu_hw_intr *intr, int irq_idx)
> 		intr->cache_irq_mask[reg_idx] = cache_irq_mask;
> 	}
> 
>-	pr_debug("%s MASK:0x%.8lx, CACHE-MASK:0x%.8x\n", dbgstr,
>+	pr_debug("DPU IRQ %d %sdisabled: MASK:0x%.8lx, CACHE-MASK:0x%.8x\n", irq_idx, dbgstr,
> 			DPU_IRQ_MASK(irq_idx), cache_irq_mask);
> 
> 	return 0;

Looks good to me.

Reviewed-by: Martin Botka <martin.botka@...ainline.org>