linux-kernel - Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

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]

Message-ID: <f6f86227-8d26-400b-9ad6-605cee966b56@linaro.org>
Date: Thu, 17 Jul 2025 12:54:55 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Dikshita Agarwal <quic_dikshita@...cinc.com>,
 Vikash Garodia <quic_vgarodia@...cinc.com>,
 Abhinav Kumar <abhinav.kumar@...ux.dev>,
 Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
 Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>
Cc: linux-media@...r.kernel.org, linux-arm-msm@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] media: iris: Add support for SM8750 (VPU v3.5)

On 17/07/2025 12:50, Dikshita Agarwal wrote:
>>>>>> +	for (i = 0; i < core->iris_platform_data->num_vpp_pipe; i++) {
>>>>>> +		ret = readl_poll_timeout(core->reg_base + VCODEC_SS_IDLE_STATUSN + 4 * i,
>>>>>> +					 val, val & 0x400000, 2000, 20000);
>>>>>> +		if (ret)
>>>>>> +			goto disable_power;
>>>>>> +	}
>>>>>> +
>>>>>> +	ret = readl_poll_timeout(core->reg_base + AON_WRAPPER_MVP_NOC_LPI_STATUS,
>>>>>> +				 val, val & BIT(0), 200, 2000);
>>>>> what are you polling here for?
>>>>
>>>>
>>>> This is not different than existing code. I don't understand why you are
>>>> commenting on something which is already there.
>>>
>>> Which code are you referring to?
>>
>> To the existing vpu33 which had Reviewed-by: Vikash Garodia
>> <quic_vgarodia@...cinc.com>
>>
>> You understand that everything here is the same, everything is a copy
>> while adding just few more things?
>>
>> My patch is not doing in this respect anything different that what you
>> reviewed.
>>
> 
> It seems to have been missed in vpu33 power off sequence as well and should
> be fixed.
> 
> Still, as mentioned earlier as well, your reference should be
> HPG/downstream driver of SM8750 not the previous generation (SM8650).

Yes and partially no, because we write upstream code matching or
extending existing upstream driver. As you said earlier, downstream is
not the truth always:

"That shouldn’t be the case. The downstream design is different, which
is why the driver requires the above code to move the GDSC"

so here I built on top of SM8650 and re-iterate whatever mistakes are
there. The best if someone fixes VPU33 and then I rebase on top,
re-using fixed code as my base.

Best regards,
Krzysztof