linux-kernel - Re: [PATCH] media: iris: Be explicit in naming of VPU2 power off handlers

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] [day] [month] [year] [list]

Message-ID: <7627d909-30d4-bf09-2736-a24bc91e28cf@quicinc.com>
Date: Wed, 2 Jul 2025 23:16:49 +0530
From: Vikash Garodia <quic_vgarodia@...cinc.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Dikshita Agarwal
	<quic_dikshita@...cinc.com>,
        Abhinav Kumar <abhinav.kumar@...ux.dev>,
        "Bryan
 O'Donoghue" <bryan.odonoghue@...aro.org>,
        Mauro Carvalho Chehab
	<mchehab@...nel.org>,
        <linux-media@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] media: iris: Be explicit in naming of VPU2 power off
 handlers


On 7/2/2025 10:59 PM, Krzysztof Kozlowski wrote:
> On 02/07/2025 19:20, Vikash Garodia wrote:
>>>  
>>>  const struct vpu_ops iris_vpu2_ops = {
>>> -	.power_off_hw = iris_vpu_power_off_hw,
>>> -	.power_off_controller = iris_vpu_power_off_controller,
>>> +	.power_off_hw = iris_vpu2_power_off_hw,
>>> +	.power_off_controller = iris_vpu2_power_off_controller,
>> There was a reason to name it as VPU* independent, as it can be used for
>> multiple VPUs. There isn't any VPU specific code within iris_vpu_power_off_hw
>> that it needs to be associated to any VPU.>>
>>>  	.calc_freq = iris_vpu2_calc_freq,
>>>  };
>>> diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
>>> index 9b7c9a1495ee..a2c8a1650153 100644
>>> --- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
>>> +++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c
>>> @@ -104,7 +104,7 @@ static void iris_vpu3_power_off_hardware(struct iris_core *core)
>>>  	writel(0x0, core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
>>>  
>>>  disable_power:
>>> -	iris_vpu_power_off_hw(core);
>>> +	iris_vpu2_power_off_hw(core);
>> Again, its like VPU3 does something specific and then reuses the common handling.
>>
>> I do not see a point in making this change.
> 
> The point is expressed in commit msg so address that. Also, this will
> not be even correct for SM8750.
When changes are raised for SM8750, the need of it can be reviewed then. Raise
the patch for power off for SM8750 to review the incorrectness better.

Regards,
Vikash
> 
>>
>> Regards,
>> Vikash
>>>  }
>>>  
>>>  static void iris_vpu33_power_off_hardware(struct iris_core *core)
>>> @@ -142,7 +142,7 @@ static void iris_vpu33_power_off_hardware(struct iris_core *core)
>>>  	writel(0x0, core->reg_base + CPU_CS_AHB_BRIDGE_SYNC_RESET);
>>>  
> 
> Please kindly trim the replies from unnecessary context. It makes it
> much easier to find new content.
> 
> Best regards,
> Krzysztof