linux-kernel - Re: [PATCH 02/13] media: qcom: camss: csiphy-3ph: Remove redundant PHY init sequence control loop

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: <e0e0ac16-2bfc-4715-a261-e25740b304cd@linaro.org>
Date: Thu, 1 Aug 2024 02:27:11 +0300
From: Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>
To: Depeng Shao <quic_depengs@...cinc.com>, rfoss@...nel.org,
 todor.too@...il.com, bryan.odonoghue@...aro.org, mchehab@...nel.org,
 robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org
Cc: quic_eberman@...cinc.com, linux-media@...r.kernel.org,
 linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, kernel@...cinc.com
Subject: Re: [PATCH 02/13] media: qcom: camss: csiphy-3ph: Remove redundant
 PHY init sequence control loop

On 7/9/24 19:06, Depeng Shao wrote:
> From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
> 
> Adding a new CSIPHY init sequence using downstream as a reference prompted
> me to look at why we are splitting up the init sequence into chunks.
> 
> Right now we declare CSI PHY init sequences as an array of five equally
> sized writes with a hard-coded control loop to iterate through each of the
> five indexes. One bug in this model is that if you don't have an even
> number of writes, you can't init the PHY as you wish.
> 
> In downstream the original code has something of the character
> phy_init_seq[MAX_LANES][MAX_PARAMS] which in upstream we have translated
> into phy_init_seq[5][SOME_NUMBER_OF_EQUAL_WRITES];
> 
> What the code does is take a pointer to the first index of the
> r = &phy_init_seq[0][0]; and then literally does write(r, value); r++;
> 
> The controlling loop that hard-codes '5' in-lieu of MAX_LANES does no
> special sleep, fabric-coherence sync or even a printk() to justify its
> existence. Our compilers are optimising all of this away anyway so lets
> drop.
> 
> Reduce the array declaration down to one flat aggregate init and let the
> code just step through. As a happy side-effect we can then also handle
> odd-number writes as the number of elements in the init sequence will no
> longer have to be evenly divisible.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
> Signed-off-by: Depeng Shao <quic_depengs@...cinc.com>
> Reviewed-by: Elliot Berman <quic_eberman@...cinc.com>

Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>

--
Best wishes,
Vladimir