linux-kernel - Re: [PATCH 2/2] phy: exynos-mipi-video: allow skipping absent PHYs

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: <2153e31b0b15d2b5b630aa4c387dc4b3@disroot.org>
Date: Mon, 07 Jul 2025 18:19:12 +0000
From: Kaustabh Chakraborty <kauschluss@...root.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I
 <kishon@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>, Neil Armstrong
 <neil.armstrong@...aro.org>, linux-phy@...ts.infradead.org,
 linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] phy: exynos-mipi-video: allow skipping absent PHYs

On 2025-07-05 08:35, Krzysztof Kozlowski wrote:
> On 26/06/2025 22:01, Kaustabh Chakraborty wrote:
>> 
>>  struct mipi_phy_device_desc {
>> -	int num_phys;
>>  	int num_regmaps;
>>  	const char *regmap_names[EXYNOS_MIPI_REGMAPS_NUM];
>>  	struct exynos_mipi_phy_desc {
>> +		bool present;
>>  		enum exynos_mipi_phy_id	coupled_phy_id;
>>  		u32 enable_val;
>>  		unsigned int enable_reg;
>> @@ -54,10 +54,9 @@ struct mipi_phy_device_desc {
>>  static const struct mipi_phy_device_desc s5pv210_mipi_phy = {
>>  	.num_regmaps = 1,
>>  	.regmap_names = {"syscon"},
>> -	.num_phys = 4,
>>  	.phys = {
>> -		{
>> -			/* EXYNOS_MIPI_PHY_ID_CSIS0 */
>> +		[EXYNOS_MIPI_PHY_ID_CSIS0] = {
> 
> 
> This should be a separate change... but overall I don't like existing
> idea and I think your change is a reason to fix actual code style 
> issue:
> 
> It is expected that each variant will define static const array and 
> then
> you assign in:
> 
> static const struct mipi_phy_device_desc exynos5420_mipi_phy = {
> 	.phys = exynos5420_mipi_phys_data
> }
> 
> which means:
> 1. You don't waste space for unused entries (now you always allocate 5
> entries, even if you have one phy)
> 2. You can count them easily - ARRAY_SIZE
> 3. Index in the array won't the the phy ID, so you need a separate ID
> member for that
> 4. You do not need this odd 'present' field, because really code which
> is not initalized should mean 'not present' and it should be never
> needed to initialize additionally to indicate 'yes, I do exist' beyond
> basic initializations.

Weird, I don't know why had I even developed this patch. The 'issue' it
fixes isn't even an issue. Oversight, I'll drop it I guess...

> 
> Best regards,
> Krzysztof