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

[Kaustabh Chakraborty <kauschluss@disroot.org>]

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
>