[PATCH 0/2] Fixes for Exynos7870 MIPI PHY support

[PATCH 0/2] Fixes for Exynos7870 MIPI PHY support

[Kaustabh Chakraborty]

A few issues have been realized after Exynos7870 MIPI PHY support got
merged [1], this patch series aims to address them.

More information about the fixes is provided in respective commit
descriptions.

[1] https://lore.kernel.org/r/20250612-exynos7870-mipi-phy-v1-0-3fff0b62d9d3@disroot.org

Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
Kaustabh Chakraborty (2):
      phy: exynos-mipi-video: correct cam0 sysreg compatible for exynos7870
      phy: exynos-mipi-video: allow skipping absent PHYs

 drivers/phy/samsung/phy-exynos-mipi-video.c | 102 ++++++++++++++++------------
 1 file changed, 58 insertions(+), 44 deletions(-)
---
base-commit: 1b152eeca84a02bdb648f16b82ef3394007a9dcf
change-id: 20250625-exynos7870-mipi-phy-fix-73e05ef89d0c

Best regards,
-- 
Kaustabh Chakraborty <kauschluss@disroot.org>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH 1/2] phy: exynos-mipi-video: correct cam0 sysreg compatible for exynos7870

[Kaustabh Chakraborty]

Fix the cam0 sysreg compatible (samsung,cam0-sysreg), which has been
erroneously declared as samsung,cam-sysreg. This follows the same
compatible name used in Exynos5433 PHY.

Fixes: 543f5e314282 ("phy: exynos-mipi-video: introduce support for exynos7870")
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
 drivers/phy/samsung/phy-exynos-mipi-video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/samsung/phy-exynos-mipi-video.c b/drivers/phy/samsung/phy-exynos-mipi-video.c
index b184923b9b400f0d536a913bdf32f3156c0a1854..1c9c340f708da0ca214639880d067706aaea8fc7 100644
--- a/drivers/phy/samsung/phy-exynos-mipi-video.c
+++ b/drivers/phy/samsung/phy-exynos-mipi-video.c
@@ -218,7 +218,7 @@ static const struct mipi_phy_device_desc exynos7870_mipi_phy = {
 	.regmap_names = {
 		"samsung,pmu-syscon",
 		"samsung,disp-sysreg",
-		"samsung,cam-sysreg"
+		"samsung,cam0-sysreg",
 	},
 	.num_phys = 4,
 	.phys = {

-- 
2.49.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 1/2] phy: exynos-mipi-video: correct cam0 sysreg compatible for exynos7870

[Krzysztof Kozlowski]

On 26/06/2025 22:01, Kaustabh Chakraborty wrote:
> Fix the cam0 sysreg compatible (samsung,cam0-sysreg), which has been
> erroneously declared as samsung,cam-sysreg. This follows the same
> compatible name used in Exynos5433 PHY.

That's not a compatible, but property name from the binding. With fixing
commit msg:


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Kaustabh Chakraborty]

In Exynos7870 MIPI PHY, DSIM1 PHY is not present. The num_phys field of
mipi_phy_device_desc when set to a value, say n, always unconditionally
initializes the first n phys, so there is no mechanism to skip over
these absent PHY blocks.

Introduce a "present" flag in all PHY descriptors which needs to be set
to true. And instead of checking the first n PHYs, check all of them. If
a certain PHY is not defined, the flag remains false and is skipped
during the init phase.

The num_phys field has to go, it has no purpose as now we're checking
all elements of the exynos_mipi_phy_desc array.

Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
 drivers/phy/samsung/phy-exynos-mipi-video.c | 100 ++++++++++++++++------------
 1 file changed, 57 insertions(+), 43 deletions(-)

diff --git a/drivers/phy/samsung/phy-exynos-mipi-video.c b/drivers/phy/samsung/phy-exynos-mipi-video.c
index 1c9c340f708da0ca214639880d067706aaea8fc7..c120c4f3531661a45374793ed7d9e9f622ff3c5d 100644
--- a/drivers/phy/samsung/phy-exynos-mipi-video.c
+++ b/drivers/phy/samsung/phy-exynos-mipi-video.c
@@ -37,10 +37,10 @@ enum exynos_mipi_phy_regmap_id {
 };
 
 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] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_DSIM0,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS4_MIPI_PHY_CONTROL(0),
@@ -65,8 +64,9 @@ static const struct mipi_phy_device_desc s5pv210_mipi_phy = {
 			.resetn_val = EXYNOS4_MIPI_PHY_SRESETN,
 			.resetn_reg = EXYNOS4_MIPI_PHY_CONTROL(0),
 			.resetn_map = EXYNOS_MIPI_REGMAP_PMU,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_DSIM0 */
+		},
+		[EXYNOS_MIPI_PHY_ID_DSIM0] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_CSIS0,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS4_MIPI_PHY_CONTROL(0),
@@ -74,8 +74,9 @@ static const struct mipi_phy_device_desc s5pv210_mipi_phy = {
 			.resetn_val = EXYNOS4_MIPI_PHY_MRESETN,
 			.resetn_reg = EXYNOS4_MIPI_PHY_CONTROL(0),
 			.resetn_map = EXYNOS_MIPI_REGMAP_PMU,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_CSIS1 */
+		},
+		[EXYNOS_MIPI_PHY_ID_CSIS1] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_DSIM1,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS4_MIPI_PHY_CONTROL(1),
@@ -83,8 +84,9 @@ static const struct mipi_phy_device_desc s5pv210_mipi_phy = {
 			.resetn_val = EXYNOS4_MIPI_PHY_SRESETN,
 			.resetn_reg = EXYNOS4_MIPI_PHY_CONTROL(1),
 			.resetn_map = EXYNOS_MIPI_REGMAP_PMU,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_DSIM1 */
+		},
+		[EXYNOS_MIPI_PHY_ID_DSIM1] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_CSIS1,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS4_MIPI_PHY_CONTROL(1),
@@ -99,10 +101,9 @@ static const struct mipi_phy_device_desc s5pv210_mipi_phy = {
 static const struct mipi_phy_device_desc exynos5420_mipi_phy = {
 	.num_regmaps = 1,
 	.regmap_names = {"syscon"},
-	.num_phys = 5,
 	.phys = {
-		{
-			/* EXYNOS_MIPI_PHY_ID_CSIS0 */
+		[EXYNOS_MIPI_PHY_ID_CSIS0] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_DSIM0,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS5420_MIPI_PHY_CONTROL(0),
@@ -110,8 +111,9 @@ static const struct mipi_phy_device_desc exynos5420_mipi_phy = {
 			.resetn_val = EXYNOS4_MIPI_PHY_SRESETN,
 			.resetn_reg = EXYNOS5420_MIPI_PHY_CONTROL(0),
 			.resetn_map = EXYNOS_MIPI_REGMAP_PMU,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_DSIM0 */
+		},
+		[EXYNOS_MIPI_PHY_ID_DSIM0] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_CSIS0,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS5420_MIPI_PHY_CONTROL(0),
@@ -119,8 +121,9 @@ static const struct mipi_phy_device_desc exynos5420_mipi_phy = {
 			.resetn_val = EXYNOS4_MIPI_PHY_MRESETN,
 			.resetn_reg = EXYNOS5420_MIPI_PHY_CONTROL(0),
 			.resetn_map = EXYNOS_MIPI_REGMAP_PMU,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_CSIS1 */
+		},
+		[EXYNOS_MIPI_PHY_ID_CSIS1] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_DSIM1,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS5420_MIPI_PHY_CONTROL(1),
@@ -128,8 +131,9 @@ static const struct mipi_phy_device_desc exynos5420_mipi_phy = {
 			.resetn_val = EXYNOS4_MIPI_PHY_SRESETN,
 			.resetn_reg = EXYNOS5420_MIPI_PHY_CONTROL(1),
 			.resetn_map = EXYNOS_MIPI_REGMAP_PMU,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_DSIM1 */
+		},
+		[EXYNOS_MIPI_PHY_ID_DSIM1] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_CSIS1,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS5420_MIPI_PHY_CONTROL(1),
@@ -137,8 +141,9 @@ static const struct mipi_phy_device_desc exynos5420_mipi_phy = {
 			.resetn_val = EXYNOS4_MIPI_PHY_MRESETN,
 			.resetn_reg = EXYNOS5420_MIPI_PHY_CONTROL(1),
 			.resetn_map = EXYNOS_MIPI_REGMAP_PMU,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_CSIS2 */
+		},
+		[EXYNOS_MIPI_PHY_ID_CSIS2] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_NONE,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS5420_MIPI_PHY_CONTROL(2),
@@ -162,10 +167,9 @@ static const struct mipi_phy_device_desc exynos5433_mipi_phy = {
 		"samsung,cam0-sysreg",
 		"samsung,cam1-sysreg"
 	},
-	.num_phys = 5,
 	.phys = {
-		{
-			/* EXYNOS_MIPI_PHY_ID_CSIS0 */
+		[EXYNOS_MIPI_PHY_ID_CSIS0] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_DSIM0,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS4_MIPI_PHY_CONTROL(0),
@@ -173,8 +177,9 @@ static const struct mipi_phy_device_desc exynos5433_mipi_phy = {
 			.resetn_val = BIT(0),
 			.resetn_reg = EXYNOS5433_SYSREG_CAM0_MIPI_DPHY_CON,
 			.resetn_map = EXYNOS_MIPI_REGMAP_CAM0,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_DSIM0 */
+		},
+		[EXYNOS_MIPI_PHY_ID_DSIM0] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_CSIS0,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS4_MIPI_PHY_CONTROL(0),
@@ -182,8 +187,9 @@ static const struct mipi_phy_device_desc exynos5433_mipi_phy = {
 			.resetn_val = BIT(0),
 			.resetn_reg = EXYNOS5433_SYSREG_DISP_MIPI_PHY,
 			.resetn_map = EXYNOS_MIPI_REGMAP_DISP,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_CSIS1 */
+		},
+		[EXYNOS_MIPI_PHY_ID_CSIS1] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_NONE,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS4_MIPI_PHY_CONTROL(1),
@@ -191,8 +197,9 @@ static const struct mipi_phy_device_desc exynos5433_mipi_phy = {
 			.resetn_val = BIT(1),
 			.resetn_reg = EXYNOS5433_SYSREG_CAM0_MIPI_DPHY_CON,
 			.resetn_map = EXYNOS_MIPI_REGMAP_CAM0,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_DSIM1 */
+		},
+		[EXYNOS_MIPI_PHY_ID_DSIM1] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_NONE,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS4_MIPI_PHY_CONTROL(1),
@@ -200,8 +207,9 @@ static const struct mipi_phy_device_desc exynos5433_mipi_phy = {
 			.resetn_val = BIT(1),
 			.resetn_reg = EXYNOS5433_SYSREG_DISP_MIPI_PHY,
 			.resetn_map = EXYNOS_MIPI_REGMAP_DISP,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_CSIS2 */
+		},
+		[EXYNOS_MIPI_PHY_ID_CSIS2] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_NONE,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS4_MIPI_PHY_CONTROL(2),
@@ -220,10 +228,9 @@ static const struct mipi_phy_device_desc exynos7870_mipi_phy = {
 		"samsung,disp-sysreg",
 		"samsung,cam0-sysreg",
 	},
-	.num_phys = 4,
 	.phys = {
-		{
-			/* EXYNOS_MIPI_PHY_ID_CSIS0 */
+		[EXYNOS_MIPI_PHY_ID_CSIS0] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_DSIM0,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS7870_MIPI_PHY_CONTROL0,
@@ -231,8 +238,9 @@ static const struct mipi_phy_device_desc exynos7870_mipi_phy = {
 			.resetn_val = BIT(0),
 			.resetn_reg = 0,
 			.resetn_map = EXYNOS_MIPI_REGMAP_CAM0,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_DSIM0 */
+		},
+		[EXYNOS_MIPI_PHY_ID_DSIM0] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_CSIS0,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS7870_MIPI_PHY_CONTROL0,
@@ -240,8 +248,9 @@ static const struct mipi_phy_device_desc exynos7870_mipi_phy = {
 			.resetn_val = BIT(0),
 			.resetn_reg = 0,
 			.resetn_map = EXYNOS_MIPI_REGMAP_DISP,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_CSIS1 */
+		},
+		[EXYNOS_MIPI_PHY_ID_CSIS1] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_NONE,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS7870_MIPI_PHY_CONTROL1,
@@ -249,8 +258,9 @@ static const struct mipi_phy_device_desc exynos7870_mipi_phy = {
 			.resetn_val = BIT(1),
 			.resetn_reg = 0,
 			.resetn_map = EXYNOS_MIPI_REGMAP_CAM0,
-		}, {
-			/* EXYNOS_MIPI_PHY_ID_CSIS2 */
+		},
+		[EXYNOS_MIPI_PHY_ID_CSIS2] = {
+			.present = true,
 			.coupled_phy_id = EXYNOS_MIPI_PHY_ID_NONE,
 			.enable_val = EXYNOS4_PHY_ENABLE,
 			.enable_reg = EXYNOS7870_MIPI_PHY_CONTROL2,
@@ -365,12 +375,15 @@ static int exynos_mipi_video_phy_probe(struct platform_device *pdev)
 		if (IS_ERR(state->regmaps[i]))
 			return PTR_ERR(state->regmaps[i]);
 	}
-	state->num_phys = phy_dev->num_phys;
+	state->num_phys = 0;
 	spin_lock_init(&state->slock);
 
 	dev_set_drvdata(dev, state);
 
-	for (i = 0; i < state->num_phys; i++) {
+	for (i = 0; i < EXYNOS_MIPI_PHYS_NUM; i++) {
+		if (!phy_dev->phys[i].present)
+			continue;
+
 		struct phy *phy = devm_phy_create(dev, NULL,
 						  &exynos_mipi_video_phy_ops);
 		if (IS_ERR(phy)) {
@@ -378,6 +391,7 @@ static int exynos_mipi_video_phy_probe(struct platform_device *pdev)
 			return PTR_ERR(phy);
 		}
 
+		state->num_phys++;
 		state->phys[i].phy = phy;
 		state->phys[i].index = i;
 		state->phys[i].data = &phy_dev->phys[i];

-- 
2.49.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Krzysztof Kozlowski]

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

[Kaustabh Chakraborty]

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy