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

[Krzysztof Kozlowski <krzk@kernel.org>]

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.

Best regards,
Krzysztof