linux-kernel - Re: [PATCH v2 02/18] media: venus: hfi_venus: Write to VIDC_CTRL

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:   Fri, 5 May 2023 20:48:51 +0200
From:   Konrad Dybcio <konrad.dybcio@...aro.org>
To:     Vikash Garodia <quic_vgarodia@...cinc.com>,
        Stanimir Varbanov <stanimir.k.varbanov@...il.com>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Dikshita Agarwal <dikshita@....qualcomm.com>,
        Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
        Mansur Alisha Shaik <mansur@...eaurora.org>,
        Jonathan Marek <jonathan@...ek.ca>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Dikshita Agarwal <quic_dikshita@...cinc.com>
Cc:     Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        Stanimir Varbanov <stanimir.varbanov@...aro.org>,
        linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        stable@...r.kernel.org
Subject: Re: [PATCH v2 02/18] media: venus: hfi_venus: Write to VIDC_CTRL_INIT
 after unmasking interrupts



On 5.05.2023 14:33, Vikash Garodia wrote:
> 
> On 5/4/2023 1:30 PM, Konrad Dybcio wrote:
>> The downstream driver signals the hardware to be enabled only after the
>> interrupts are unmasked, which... makes sense. Follow suit.
> 
> Rephrase the commit text,
> 
> 1. No need to mention downstream driver, if something is buggy, fix it.
Generally I'd agree, however in this specific case the downstream
driver is the only available source of knowledge about what the correct
(or at least working) initialization sequence of this hw block is.

> 
> 2. Avoid "..." and lets make it more formal.
Ack

Konrad
> 
>> Cc: stable@...r.kernel.org # v4.12+
>> Fixes: d96d3f30c0f2 ("[media] media: venus: hfi: add Venus HFI files")
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@...aro.org>
>> ---
>>   drivers/media/platform/qcom/venus/hfi_venus.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c
>> index bff435abd59b..8fc8f46dc390 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_venus.c
>> +++ b/drivers/media/platform/qcom/venus/hfi_venus.c
>> @@ -453,7 +453,6 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
>>       void __iomem *wrapper_base = hdev->core->wrapper_base;
>>       int ret = 0;
>>   -    writel(BIT(VIDC_CTRL_INIT_CTRL_SHIFT), cpu_cs_base + VIDC_CTRL_INIT);
>>       if (IS_V6(hdev->core)) {
>>           mask_val = readl(wrapper_base + WRAPPER_INTR_MASK);
>>           mask_val &= ~(WRAPPER_INTR_MASK_A2HWD_BASK_V6 |
>> @@ -464,6 +463,7 @@ static int venus_boot_core(struct venus_hfi_device *hdev)
>>       writel(mask_val, wrapper_base + WRAPPER_INTR_MASK);
>>       writel(1, cpu_cs_base + CPU_CS_SCIACMDARG3);
>>   +    writel(BIT(VIDC_CTRL_INIT_CTRL_SHIFT), cpu_cs_base + VIDC_CTRL_INIT);
>>       while (!ctrl_status && count < max_tries) {
>>           ctrl_status = readl(cpu_cs_base + CPU_CS_SCIACMDARG0);
>>           if ((ctrl_status & CPU_CS_SCIACMDARG0_ERROR_STATUS_MASK) == 4) {
> 
> Above code looks good.
> 
> -Vikash
>