* [PATCH 0/2] Fixes for Exynos7870 MIPI PHY support
@ 2025-06-26 20:01 Kaustabh Chakraborty
2025-06-26 20:01 ` [PATCH 1/2] phy: exynos-mipi-video: correct cam0 sysreg compatible for exynos7870 Kaustabh Chakraborty
2025-06-26 20:01 ` [PATCH 2/2] phy: exynos-mipi-video: allow skipping absent PHYs Kaustabh Chakraborty
0 siblings, 2 replies; 6+ messages in thread
From: Kaustabh Chakraborty @ 2025-06-26 20:01 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Krzysztof Kozlowski,
Alim Akhtar, Neil Armstrong
Cc: linux-phy, linux-arm-kernel, linux-samsung-soc, linux-kernel,
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] phy: exynos-mipi-video: correct cam0 sysreg compatible for exynos7870
2025-06-26 20:01 [PATCH 0/2] Fixes for Exynos7870 MIPI PHY support Kaustabh Chakraborty
@ 2025-06-26 20:01 ` Kaustabh Chakraborty
2025-07-05 8:26 ` Krzysztof Kozlowski
2025-06-26 20:01 ` [PATCH 2/2] phy: exynos-mipi-video: allow skipping absent PHYs Kaustabh Chakraborty
1 sibling, 1 reply; 6+ messages in thread
From: Kaustabh Chakraborty @ 2025-06-26 20:01 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Krzysztof Kozlowski,
Alim Akhtar, Neil Armstrong
Cc: linux-phy, linux-arm-kernel, linux-samsung-soc, linux-kernel,
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] phy: exynos-mipi-video: allow skipping absent PHYs
2025-06-26 20:01 [PATCH 0/2] Fixes for Exynos7870 MIPI PHY support Kaustabh Chakraborty
2025-06-26 20:01 ` [PATCH 1/2] phy: exynos-mipi-video: correct cam0 sysreg compatible for exynos7870 Kaustabh Chakraborty
@ 2025-06-26 20:01 ` Kaustabh Chakraborty
2025-07-05 8:35 ` Krzysztof Kozlowski
1 sibling, 1 reply; 6+ messages in thread
From: Kaustabh Chakraborty @ 2025-06-26 20:01 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Krzysztof Kozlowski,
Alim Akhtar, Neil Armstrong
Cc: linux-phy, linux-arm-kernel, linux-samsung-soc, linux-kernel,
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] phy: exynos-mipi-video: correct cam0 sysreg compatible for exynos7870
2025-06-26 20:01 ` [PATCH 1/2] phy: exynos-mipi-video: correct cam0 sysreg compatible for exynos7870 Kaustabh Chakraborty
@ 2025-07-05 8:26 ` Krzysztof Kozlowski
0 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-05 8:26 UTC (permalink / raw)
To: Kaustabh Chakraborty, Vinod Koul, Kishon Vijay Abraham I,
Alim Akhtar, Neil Armstrong
Cc: linux-phy, linux-arm-kernel, linux-samsung-soc, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] phy: exynos-mipi-video: allow skipping absent PHYs
2025-06-26 20:01 ` [PATCH 2/2] phy: exynos-mipi-video: allow skipping absent PHYs Kaustabh Chakraborty
@ 2025-07-05 8:35 ` Krzysztof Kozlowski
2025-07-07 18:19 ` Kaustabh Chakraborty
0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-05 8:35 UTC (permalink / raw)
To: Kaustabh Chakraborty, Vinod Koul, Kishon Vijay Abraham I,
Alim Akhtar, Neil Armstrong
Cc: linux-phy, linux-arm-kernel, linux-samsung-soc, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] phy: exynos-mipi-video: allow skipping absent PHYs
2025-07-05 8:35 ` Krzysztof Kozlowski
@ 2025-07-07 18:19 ` Kaustabh Chakraborty
0 siblings, 0 replies; 6+ messages in thread
From: Kaustabh Chakraborty @ 2025-07-07 18:19 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Vinod Koul, Kishon Vijay Abraham I, Alim Akhtar, Neil Armstrong,
linux-phy, linux-arm-kernel, linux-samsung-soc, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-07-07 18:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-26 20:01 [PATCH 0/2] Fixes for Exynos7870 MIPI PHY support Kaustabh Chakraborty
2025-06-26 20:01 ` [PATCH 1/2] phy: exynos-mipi-video: correct cam0 sysreg compatible for exynos7870 Kaustabh Chakraborty
2025-07-05 8:26 ` Krzysztof Kozlowski
2025-06-26 20:01 ` [PATCH 2/2] phy: exynos-mipi-video: allow skipping absent PHYs Kaustabh Chakraborty
2025-07-05 8:35 ` Krzysztof Kozlowski
2025-07-07 18:19 ` Kaustabh Chakraborty
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).