[PATCH v3 10/13] drm/bridge: samsung-dsim: add ability to define clock names for every variant

Kaustabh Chakraborty posted 13 patches 3 months ago
[PATCH v3 10/13] drm/bridge: samsung-dsim: add ability to define clock names for every variant
Posted by Kaustabh Chakraborty 3 months ago
Presently, all devices refer to clock names from a single array. The
only controlling parameter is the number of clocks (num_clks field of
samsung_dsim_driver_data) which uses the first n clocks of that array.
As new devices are added, this approach turns out to be cumbersome.

Separate the clock names in individual arrays required by each variant,
in a struct clk_bulk_data. Add a pointer field to the driver data struct
which points to their respective clock names, and rework the clock usage
code to use the clk_bulk_* API instead.

Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 88 +++++++++++++++++------------------
 include/drm/bridge/samsung-dsim.h     |  2 +-
 2 files changed, 44 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 4b49707730db76aa8fd3ab973b02507436750889..b6b3bbcbd0f438e5e1d3faf18f8c2d532a4ecc93 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -219,23 +219,31 @@
 #define DSI_XFER_TIMEOUT_MS		100
 #define DSI_RX_FIFO_EMPTY		0x30800002
 
-#define OLD_SCLK_MIPI_CLK_NAME		"pll_clk"
-
 #define PS_TO_CYCLE(ps, hz) DIV64_U64_ROUND_CLOSEST(((ps) * (hz)), 1000000000000ULL)
 
-static const char *const clk_names[5] = {
-	"bus_clk",
-	"sclk_mipi",
-	"phyclk_mipidphy0_bitclkdiv8",
-	"phyclk_mipidphy0_rxclkesc0",
-	"sclk_rgb_vclk_to_dsim0"
-};
-
 enum samsung_dsim_transfer_type {
 	EXYNOS_DSI_TX,
 	EXYNOS_DSI_RX,
 };
 
+static struct clk_bulk_data exynos3_clk_bulk_data[] = {
+	{ .id = "bus_clk" },
+	{ .id = "pll_clk" },
+};
+
+static struct clk_bulk_data exynos4_clk_bulk_data[] = {
+	{ .id = "bus_clk" },
+	{ .id = "sclk_mipi" },
+};
+
+static struct clk_bulk_data exynos5433_clk_bulk_data[] = {
+	{ .id = "bus_clk" },
+	{ .id = "sclk_mipi" },
+	{ .id = "phyclk_mipidphy0_bitclkdiv8" },
+	{ .id = "phyclk_mipidphy0_rxclkesc0" },
+	{ .id = "sclk_rgb_vclk_to_dsim0" },
+};
+
 enum reg_idx {
 	DSIM_STATUS_REG,	/* Status register (legacy) */
 	DSIM_LINK_STATUS_REG,	/* Link status register */
@@ -408,7 +416,8 @@ static const struct samsung_dsim_driver_data exynos3_dsi_driver_data = {
 	.has_legacy_status_reg = 1,
 	.has_freqband = 1,
 	.has_clklane_stop = 1,
-	.num_clks = 2,
+	.clk_data = exynos3_clk_bulk_data,
+	.num_clks = ARRAY_SIZE(exynos3_clk_bulk_data),
 	.max_freq = 1000,
 	.wait_for_hdr_fifo = 1,
 	.wait_for_reset = 1,
@@ -439,7 +448,8 @@ static const struct samsung_dsim_driver_data exynos4_dsi_driver_data = {
 	.has_legacy_status_reg = 1,
 	.has_freqband = 1,
 	.has_clklane_stop = 1,
-	.num_clks = 2,
+	.clk_data = exynos4_clk_bulk_data,
+	.num_clks = ARRAY_SIZE(exynos4_clk_bulk_data),
 	.max_freq = 1000,
 	.wait_for_hdr_fifo = 1,
 	.wait_for_reset = 1,
@@ -468,7 +478,8 @@ static const struct samsung_dsim_driver_data exynos5_dsi_driver_data = {
 	.reg_ofs = exynos_reg_ofs,
 	.plltmr_reg = 0x58,
 	.has_legacy_status_reg = 1,
-	.num_clks = 2,
+	.clk_data = exynos3_clk_bulk_data,
+	.num_clks = ARRAY_SIZE(exynos3_clk_bulk_data),
 	.max_freq = 1000,
 	.wait_for_hdr_fifo = 1,
 	.wait_for_reset = 1,
@@ -497,7 +508,8 @@ static const struct samsung_dsim_driver_data exynos5433_dsi_driver_data = {
 	.plltmr_reg = 0xa0,
 	.has_legacy_status_reg = 1,
 	.has_clklane_stop = 1,
-	.num_clks = 5,
+	.clk_data = exynos5433_clk_bulk_data,
+	.num_clks = ARRAY_SIZE(exynos5433_clk_bulk_data),
 	.max_freq = 1500,
 	.wait_for_hdr_fifo = 1,
 	.wait_for_reset = 0,
@@ -526,7 +538,8 @@ static const struct samsung_dsim_driver_data exynos5422_dsi_driver_data = {
 	.plltmr_reg = 0xa0,
 	.has_legacy_status_reg = 1,
 	.has_clklane_stop = 1,
-	.num_clks = 2,
+	.clk_data = exynos3_clk_bulk_data,
+	.num_clks = ARRAY_SIZE(exynos3_clk_bulk_data),
 	.max_freq = 1500,
 	.wait_for_hdr_fifo = 1,
 	.wait_for_reset = 1,
@@ -555,7 +568,8 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
 	.plltmr_reg = 0xa0,
 	.has_legacy_status_reg = 1,
 	.has_clklane_stop = 1,
-	.num_clks = 2,
+	.clk_data = exynos4_clk_bulk_data,
+	.num_clks = ARRAY_SIZE(exynos4_clk_bulk_data),
 	.max_freq = 2100,
 	.wait_for_hdr_fifo = 1,
 	.wait_for_reset = 0,
@@ -2021,7 +2035,7 @@ int samsung_dsim_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct samsung_dsim *dsi;
-	int ret, i;
+	int ret;
 
 	dsi = devm_drm_bridge_alloc(dev, struct samsung_dsim, bridge, &samsung_dsim_bridge_funcs);
 	if (IS_ERR(dsi))
@@ -2045,23 +2059,11 @@ int samsung_dsim_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to get regulators\n");
 
-	dsi->clks = devm_kcalloc(dev, dsi->driver_data->num_clks,
-				 sizeof(*dsi->clks), GFP_KERNEL);
-	if (!dsi->clks)
-		return -ENOMEM;
-
-	for (i = 0; i < dsi->driver_data->num_clks; i++) {
-		dsi->clks[i] = devm_clk_get(dev, clk_names[i]);
-		if (IS_ERR(dsi->clks[i])) {
-			if (strcmp(clk_names[i], "sclk_mipi") == 0) {
-				dsi->clks[i] = devm_clk_get(dev, OLD_SCLK_MIPI_CLK_NAME);
-				if (!IS_ERR(dsi->clks[i]))
-					continue;
-			}
-
-			dev_info(dev, "failed to get the clock: %s\n", clk_names[i]);
-			return PTR_ERR(dsi->clks[i]);
-		}
+	ret = devm_clk_bulk_get(dev, dsi->driver_data->num_clks,
+				dsi->driver_data->clk_data);
+	if (ret) {
+		dev_err(dev, "failed to get clocks in bulk (%d)\n", ret);
+		return ret;
 	}
 
 	dsi->reg_base = devm_platform_ioremap_resource(pdev, 0);
@@ -2134,7 +2136,7 @@ static int samsung_dsim_suspend(struct device *dev)
 {
 	struct samsung_dsim *dsi = dev_get_drvdata(dev);
 	const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
-	int ret, i;
+	int ret;
 
 	usleep_range(10000, 20000);
 
@@ -2150,8 +2152,7 @@ static int samsung_dsim_suspend(struct device *dev)
 
 	phy_power_off(dsi->phy);
 
-	for (i = driver_data->num_clks - 1; i > -1; i--)
-		clk_disable_unprepare(dsi->clks[i]);
+	clk_bulk_disable_unprepare(driver_data->num_clks, driver_data->clk_data);
 
 	ret = regulator_bulk_disable(ARRAY_SIZE(dsi->supplies), dsi->supplies);
 	if (ret < 0)
@@ -2164,7 +2165,7 @@ static int samsung_dsim_resume(struct device *dev)
 {
 	struct samsung_dsim *dsi = dev_get_drvdata(dev);
 	const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
-	int ret, i;
+	int ret;
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(dsi->supplies), dsi->supplies);
 	if (ret < 0) {
@@ -2172,11 +2173,9 @@ static int samsung_dsim_resume(struct device *dev)
 		return ret;
 	}
 
-	for (i = 0; i < driver_data->num_clks; i++) {
-		ret = clk_prepare_enable(dsi->clks[i]);
-		if (ret < 0)
-			goto err_clk;
-	}
+	ret = clk_bulk_prepare_enable(driver_data->num_clks, driver_data->clk_data);
+	if (ret < 0)
+		goto err_clk;
 
 	ret = phy_power_on(dsi->phy);
 	if (ret < 0) {
@@ -2187,8 +2186,7 @@ static int samsung_dsim_resume(struct device *dev)
 	return 0;
 
 err_clk:
-	while (--i > -1)
-		clk_disable_unprepare(dsi->clks[i]);
+	clk_bulk_disable_unprepare(driver_data->num_clks, driver_data->clk_data);
 	regulator_bulk_disable(ARRAY_SIZE(dsi->supplies), dsi->supplies);
 
 	return ret;
diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h
index 04ed11787bbd22503f7221cad1a491a4e5e66781..eb9fdbab1b34074923daa0aa0443c33c5b99ae42 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -58,6 +58,7 @@ struct samsung_dsim_driver_data {
 	unsigned int has_clklane_stop:1;
 	unsigned int has_broken_fifoctrl_emptyhdr:1;
 	unsigned int has_sfrctrl:1;
+	struct clk_bulk_data *clk_data;
 	unsigned int num_clks;
 	unsigned int min_freq;
 	unsigned int max_freq;
@@ -104,7 +105,6 @@ struct samsung_dsim {
 
 	void __iomem *reg_base;
 	struct phy *phy;
-	struct clk **clks;
 	struct clk *pll_clk;
 	struct regulator_bulk_data supplies[2];
 	int irq;

-- 
2.49.0
Re: [PATCH v3 10/13] drm/bridge: samsung-dsim: add ability to define clock names for every variant
Posted by Inki Dae 2 months, 1 week ago
2025년 7월 7일 (월) 오전 3:28, Kaustabh Chakraborty <kauschluss@disroot.org>님이 작성:
>
> Presently, all devices refer to clock names from a single array. The
> only controlling parameter is the number of clocks (num_clks field of
> samsung_dsim_driver_data) which uses the first n clocks of that array.
> As new devices are added, this approach turns out to be cumbersome.
>
> Separate the clock names in individual arrays required by each variant,
> in a struct clk_bulk_data. Add a pointer field to the driver data struct
> which points to their respective clock names, and rework the clock usage
> code to use the clk_bulk_* API instead.
>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
> ---
>  drivers/gpu/drm/bridge/samsung-dsim.c | 88 +++++++++++++++++------------------
>  include/drm/bridge/samsung-dsim.h     |  2 +-
>  2 files changed, 44 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 4b49707730db76aa8fd3ab973b02507436750889..b6b3bbcbd0f438e5e1d3faf18f8c2d532a4ecc93 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -219,23 +219,31 @@
>  #define DSI_XFER_TIMEOUT_MS            100
>  #define DSI_RX_FIFO_EMPTY              0x30800002
>
> -#define OLD_SCLK_MIPI_CLK_NAME         "pll_clk"
> -
>  #define PS_TO_CYCLE(ps, hz) DIV64_U64_ROUND_CLOSEST(((ps) * (hz)), 1000000000000ULL)
>
> -static const char *const clk_names[5] = {
> -       "bus_clk",
> -       "sclk_mipi",
> -       "phyclk_mipidphy0_bitclkdiv8",
> -       "phyclk_mipidphy0_rxclkesc0",
> -       "sclk_rgb_vclk_to_dsim0"
> -};
> -
>  enum samsung_dsim_transfer_type {
>         EXYNOS_DSI_TX,
>         EXYNOS_DSI_RX,
>  };
>
> +static struct clk_bulk_data exynos3_clk_bulk_data[] = {
> +       { .id = "bus_clk" },
> +       { .id = "pll_clk" },
> +};
> +
> +static struct clk_bulk_data exynos4_clk_bulk_data[] = {
> +       { .id = "bus_clk" },
> +       { .id = "sclk_mipi" },
> +};
> +
> +static struct clk_bulk_data exynos5433_clk_bulk_data[] = {
> +       { .id = "bus_clk" },
> +       { .id = "sclk_mipi" },
> +       { .id = "phyclk_mipidphy0_bitclkdiv8" },
> +       { .id = "phyclk_mipidphy0_rxclkesc0" },
> +       { .id = "sclk_rgb_vclk_to_dsim0" },
> +};
> +
>  enum reg_idx {
>         DSIM_STATUS_REG,        /* Status register (legacy) */
>         DSIM_LINK_STATUS_REG,   /* Link status register */
> @@ -408,7 +416,8 @@ static const struct samsung_dsim_driver_data exynos3_dsi_driver_data = {
>         .has_legacy_status_reg = 1,
>         .has_freqband = 1,
>         .has_clklane_stop = 1,
> -       .num_clks = 2,
> +       .clk_data = exynos3_clk_bulk_data,
> +       .num_clks = ARRAY_SIZE(exynos3_clk_bulk_data),
>         .max_freq = 1000,
>         .wait_for_hdr_fifo = 1,
>         .wait_for_reset = 1,
> @@ -439,7 +448,8 @@ static const struct samsung_dsim_driver_data exynos4_dsi_driver_data = {
>         .has_legacy_status_reg = 1,
>         .has_freqband = 1,
>         .has_clklane_stop = 1,
> -       .num_clks = 2,
> +       .clk_data = exynos4_clk_bulk_data,
> +       .num_clks = ARRAY_SIZE(exynos4_clk_bulk_data),
>         .max_freq = 1000,
>         .wait_for_hdr_fifo = 1,
>         .wait_for_reset = 1,
> @@ -468,7 +478,8 @@ static const struct samsung_dsim_driver_data exynos5_dsi_driver_data = {
>         .reg_ofs = exynos_reg_ofs,
>         .plltmr_reg = 0x58,
>         .has_legacy_status_reg = 1,
> -       .num_clks = 2,
> +       .clk_data = exynos3_clk_bulk_data,
> +       .num_clks = ARRAY_SIZE(exynos3_clk_bulk_data),
>         .max_freq = 1000,
>         .wait_for_hdr_fifo = 1,
>         .wait_for_reset = 1,
> @@ -497,7 +508,8 @@ static const struct samsung_dsim_driver_data exynos5433_dsi_driver_data = {
>         .plltmr_reg = 0xa0,
>         .has_legacy_status_reg = 1,
>         .has_clklane_stop = 1,
> -       .num_clks = 5,
> +       .clk_data = exynos5433_clk_bulk_data,
> +       .num_clks = ARRAY_SIZE(exynos5433_clk_bulk_data),
>         .max_freq = 1500,
>         .wait_for_hdr_fifo = 1,
>         .wait_for_reset = 0,
> @@ -526,7 +538,8 @@ static const struct samsung_dsim_driver_data exynos5422_dsi_driver_data = {
>         .plltmr_reg = 0xa0,
>         .has_legacy_status_reg = 1,
>         .has_clklane_stop = 1,
> -       .num_clks = 2,
> +       .clk_data = exynos3_clk_bulk_data,
> +       .num_clks = ARRAY_SIZE(exynos3_clk_bulk_data),
>         .max_freq = 1500,
>         .wait_for_hdr_fifo = 1,
>         .wait_for_reset = 1,
> @@ -555,7 +568,8 @@ static const struct samsung_dsim_driver_data imx8mm_dsi_driver_data = {
>         .plltmr_reg = 0xa0,
>         .has_legacy_status_reg = 1,
>         .has_clklane_stop = 1,
> -       .num_clks = 2,
> +       .clk_data = exynos4_clk_bulk_data,
> +       .num_clks = ARRAY_SIZE(exynos4_clk_bulk_data),
>         .max_freq = 2100,
>         .wait_for_hdr_fifo = 1,
>         .wait_for_reset = 0,
> @@ -2021,7 +2035,7 @@ int samsung_dsim_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
>         struct samsung_dsim *dsi;
> -       int ret, i;
> +       int ret;
>
>         dsi = devm_drm_bridge_alloc(dev, struct samsung_dsim, bridge, &samsung_dsim_bridge_funcs);
>         if (IS_ERR(dsi))
> @@ -2045,23 +2059,11 @@ int samsung_dsim_probe(struct platform_device *pdev)
>         if (ret)
>                 return dev_err_probe(dev, ret, "failed to get regulators\n");
>
> -       dsi->clks = devm_kcalloc(dev, dsi->driver_data->num_clks,
> -                                sizeof(*dsi->clks), GFP_KERNEL);
> -       if (!dsi->clks)
> -               return -ENOMEM;
> -
> -       for (i = 0; i < dsi->driver_data->num_clks; i++) {
> -               dsi->clks[i] = devm_clk_get(dev, clk_names[i]);
> -               if (IS_ERR(dsi->clks[i])) {
> -                       if (strcmp(clk_names[i], "sclk_mipi") == 0) {
> -                               dsi->clks[i] = devm_clk_get(dev, OLD_SCLK_MIPI_CLK_NAME);
> -                               if (!IS_ERR(dsi->clks[i]))
> -                                       continue;
> -                       }
> -
> -                       dev_info(dev, "failed to get the clock: %s\n", clk_names[i]);
> -                       return PTR_ERR(dsi->clks[i]);
> -               }
> +       ret = devm_clk_bulk_get(dev, dsi->driver_data->num_clks,
> +                               dsi->driver_data->clk_data);
> +       if (ret) {
> +               dev_err(dev, "failed to get clocks in bulk (%d)\n", ret);
> +               return ret;

Above change modifies the existing behavior.

Previously, when devm_clk_get() failed and the clock name was
"sclk_mipi", the code included a fallback mechanism to try "pll_clk"
instead. This fallback logic has been removed in the current patch.

While changing this behavior may raise concerns, the benefits of
refactoring—specifically, defining clock names per SoC and adopting
the clk_bulk_* API for improved maintainability—appear to outweigh the
potential downsides.

Unless there are objections from other reviewers, I intend to proceed
with merging this patch.
If anyone has concerns or sees potential issues with this change,
please share your thoughts.

Thanks,
Inki Dae

>         }
>
>         dsi->reg_base = devm_platform_ioremap_resource(pdev, 0);
> @@ -2134,7 +2136,7 @@ static int samsung_dsim_suspend(struct device *dev)
>  {
>         struct samsung_dsim *dsi = dev_get_drvdata(dev);
>         const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
> -       int ret, i;
> +       int ret;
>
>         usleep_range(10000, 20000);
>
> @@ -2150,8 +2152,7 @@ static int samsung_dsim_suspend(struct device *dev)
>
>         phy_power_off(dsi->phy);
>
> -       for (i = driver_data->num_clks - 1; i > -1; i--)
> -               clk_disable_unprepare(dsi->clks[i]);
> +       clk_bulk_disable_unprepare(driver_data->num_clks, driver_data->clk_data);
>
>         ret = regulator_bulk_disable(ARRAY_SIZE(dsi->supplies), dsi->supplies);
>         if (ret < 0)
> @@ -2164,7 +2165,7 @@ static int samsung_dsim_resume(struct device *dev)
>  {
>         struct samsung_dsim *dsi = dev_get_drvdata(dev);
>         const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
> -       int ret, i;
> +       int ret;
>
>         ret = regulator_bulk_enable(ARRAY_SIZE(dsi->supplies), dsi->supplies);
>         if (ret < 0) {
> @@ -2172,11 +2173,9 @@ static int samsung_dsim_resume(struct device *dev)
>                 return ret;
>         }
>
> -       for (i = 0; i < driver_data->num_clks; i++) {
> -               ret = clk_prepare_enable(dsi->clks[i]);
> -               if (ret < 0)
> -                       goto err_clk;
> -       }
> +       ret = clk_bulk_prepare_enable(driver_data->num_clks, driver_data->clk_data);
> +       if (ret < 0)
> +               goto err_clk;
>
>         ret = phy_power_on(dsi->phy);
>         if (ret < 0) {
> @@ -2187,8 +2186,7 @@ static int samsung_dsim_resume(struct device *dev)
>         return 0;
>
>  err_clk:
> -       while (--i > -1)
> -               clk_disable_unprepare(dsi->clks[i]);
> +       clk_bulk_disable_unprepare(driver_data->num_clks, driver_data->clk_data);
>         regulator_bulk_disable(ARRAY_SIZE(dsi->supplies), dsi->supplies);
>
>         return ret;
> diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h
> index 04ed11787bbd22503f7221cad1a491a4e5e66781..eb9fdbab1b34074923daa0aa0443c33c5b99ae42 100644
> --- a/include/drm/bridge/samsung-dsim.h
> +++ b/include/drm/bridge/samsung-dsim.h
> @@ -58,6 +58,7 @@ struct samsung_dsim_driver_data {
>         unsigned int has_clklane_stop:1;
>         unsigned int has_broken_fifoctrl_emptyhdr:1;
>         unsigned int has_sfrctrl:1;
> +       struct clk_bulk_data *clk_data;
>         unsigned int num_clks;
>         unsigned int min_freq;
>         unsigned int max_freq;
> @@ -104,7 +105,6 @@ struct samsung_dsim {
>
>         void __iomem *reg_base;
>         struct phy *phy;
> -       struct clk **clks;
>         struct clk *pll_clk;
>         struct regulator_bulk_data supplies[2];
>         int irq;
>
> --
> 2.49.0
>
>
Re: [PATCH v3 10/13] drm/bridge: samsung-dsim: add ability to define clock names for every variant
Posted by Kaustabh Chakraborty 2 months, 1 week ago
On 2025-07-29 07:02, Inki Dae wrote:
> 2025년 7월 7일 (월) 오전 3:28, Kaustabh Chakraborty 
> <kauschluss@disroot.org>님이 작성:
>> 
>> -       dsi->clks = devm_kcalloc(dev, dsi->driver_data->num_clks,
>> -                                sizeof(*dsi->clks), GFP_KERNEL);
>> -       if (!dsi->clks)
>> -               return -ENOMEM;
>> -
>> -       for (i = 0; i < dsi->driver_data->num_clks; i++) {
>> -               dsi->clks[i] = devm_clk_get(dev, clk_names[i]);
>> -               if (IS_ERR(dsi->clks[i])) {
>> -                       if (strcmp(clk_names[i], "sclk_mipi") == 0) {
>> -                               dsi->clks[i] = devm_clk_get(dev, 
>> OLD_SCLK_MIPI_CLK_NAME);
>> -                               if (!IS_ERR(dsi->clks[i]))
>> -                                       continue;
>> -                       }
>> -
>> -                       dev_info(dev, "failed to get the clock: %s\n", 
>> clk_names[i]);
>> -                       return PTR_ERR(dsi->clks[i]);
>> -               }
>> +       ret = devm_clk_bulk_get(dev, dsi->driver_data->num_clks,
>> +                               dsi->driver_data->clk_data);
>> +       if (ret) {
>> +               dev_err(dev, "failed to get clocks in bulk (%d)\n", 
>> ret);
>> +               return ret;
> 
> Above change modifies the existing behavior.
> 
> Previously, when devm_clk_get() failed and the clock name was
> "sclk_mipi", the code included a fallback mechanism to try "pll_clk"
> instead. This fallback logic has been removed in the current patch.
> 
> While changing this behavior may raise concerns, the benefits of
> refactoring—specifically, defining clock names per SoC and adopting
> the clk_bulk_* API for improved maintainability—appear to outweigh the
> potential downsides.

I have checked all devices which use this driver.

Here is a mapping of all compatible -> clock names in the driver:
- fsl,imx8mm-mipi-dsim: bus_clk, sclk_mipi
- fsl,imx8mp-mipi-dsim: bus_clk, sclk_mipi
- samsung,exynos3250-mipi-dsi: bus_clk, pll_clk
- samsung,exynos4210-mipi-dsi: bus_clk, sclk_mipi
- samsung,exynos5410-mipi-dsi: bus_clk, pll_clk
- samsung,exynos5422-mipi-dsi: bus_clk, pll_clk
- samsung,exynos5433-mipi-dsi: bus_clk, sclk_mipi,
                                phyclk_mipidphy0_bitclkdiv8,
                                phyclk_mipidphy0_rxclkesc0,
                                sclk_rgb_vclk_to_dsim0

And here is what I found by grep-ing all devicetrees:

arm/boot/dts/nxp/imx/imx7s.dtsi
     compatible = "fsl,imx7d-mipi-dsim", "fsl,imx8mm-mipi-dsim";
     (uses bus_clk, sclk_mipi)

arm/boot/dts/samsung/exynos3250.dtsi
     compatible = "samsung,exynos3250-mipi-dsi";
     (uses bus_clk, pll_clk)

arm/boot/dts/samsung/exynos4.dtsi
     compatible = "samsung,exynos4210-mipi-dsi";
     (uses bus_clk, sclk_mipi)

arm/boot/dts/samsung/exynos5420.dtsi
     compatible = "samsung,exynos5410-mipi-dsi";
     (uses bus_clk, pll_clk)

arm/boot/dts/samsung/exynos5800.dtsi
     compatible = "samsung,exynos5422-mipi-dsi";
     (uses bus_clk, pll_clk - uses node from exynos5420.dtsi)

arm64/boot/dts/exynos/exynos5433.dtsi
     compatible = "samsung,exynos5433-mipi-dsi";
     (uses bus_clk, sclk_mipi, and 3 others as mentioned above)

arm64/boot/dts/freescale/imx8mm.dtsi
     compatible = "fsl,imx8mm-mipi-dsim";
     (uses bus_clk, sclk_mipi)

arm64/boot/dts/freescale/imx8mn.dtsi
     compatible = "fsl,imx8mn-mipi-dsim", "fsl,imx8mm-mipi-dsim";
     (uses bus_clk, sclk_mipi)

arm64/boot/dts/freescale/imx8mp.dtsi
     compatible = "fsl,imx8mp-mipi-dsim";
     (uses bus_clk, sclk_mipi)

So, there shouldn't be any regressions.

> 
> Unless there are objections from other reviewers, I intend to proceed
> with merging this patch.
> If anyone has concerns or sees potential issues with this change,
> please share your thoughts.
> 
> Thanks,
> Inki Dae
>