[PATCH v2 0/5] Add supply property for DSI controller

[PATCH v2 0/5] Add supply property for DSI controller

[Yannick Fertré]

The DSI controller needs a new property that powers its physical layer.
Binding has been updated to documented this property.
Device tree of stm32mp157c soc.
Move reg18 & reg11 to stm32mp157c device tree file.
Remove property phy-dsi-supply property to stm32mp157c-dk2.dts file.


Changes in v2:
- rename patch drm/stm: dsi: add support of an optional regulator
- rework dw_mipi_dsi-stm probe sequence

Yannick Fertré (5):
  dt-bindings: display: stm32: add supply property to DSI controller
  drm/stm: dsi: add support of an optional regulator
  ARM: dts: stm32: add phy-dsi-supply property on stm32mp157c
  ARM: dts: stm32: move fixe regulators reg11 & reg18
  ARM: dts: stm32: remove phy-dsi-supply property on stm32mp157c-dk2
    board

 .../devicetree/bindings/display/st,stm32-ltdc.txt    |  3 +++
 arch/arm/boot/dts/stm32mp157c-dk2.dts                |  9 ---------
 arch/arm/boot/dts/stm32mp157c-ed1.dts                | 16 ----------------
 arch/arm/boot/dts/stm32mp157c.dtsi                   | 17 +++++++++++++++++
 drivers/gpu/drm/stm/dw_mipi_dsi-stm.c              | 45 +++++++++++++++++++---
 5 files changed, 40 insertions(+), 25 deletions(-)

--
2.7.4

[PATCH v2 1/5] dt-bindings: display: stm32: add supply property to DSI controller

[Yannick Fertré]

This patch adds documentation of a new property phy-dsi-supply to the
STM32 DSI controller.

Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
---
 Documentation/devicetree/bindings/display/st,stm32-ltdc.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt b/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
index 3eb1b48..60c54da 100644
--- a/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
+++ b/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
@@ -40,6 +40,8 @@ Mandatory nodes specific to STM32 DSI:
 - panel or bridge node: A node containing the panel or bridge description as
   documented in [6].
   - port: panel or bridge port node, connected to the DSI output port (port@1).
+Optional properties:
+- phy-dsi-supply: phandle of the regulator that provides the supply voltage.
 
 Note: You can find more documentation in the following references
 [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
@@ -101,6 +103,7 @@ Example 2: DSI panel
 			clock-names = "pclk", "ref";
 			resets = <&rcc STM32F4_APB2_RESET(DSI)>;
 			reset-names = "apb";
+			phy-dsi-supply = <&reg18>;
 
 			ports {
 				#address-cells = <1>;
-- 
2.7.4

[PATCH v2 2/3] drm/stm: dsi: add support of an optional regulator

[Yannick Fertré]

Add support of an optional regulator for the phy part of the DSI
controller.

Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
---
 drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 45 ++++++++++++++++++++++++++++++-----
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
index 1bef73e..22bd095 100644
--- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
+++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
@@ -9,6 +9,7 @@
 #include <linux/clk.h>
 #include <linux/iopoll.h>
 #include <linux/module.h>
+#include <linux/regulator/consumer.h>
 #include <drm/drmP.h>
 #include <drm/drm_mipi_dsi.h>
 #include <drm/bridge/dw_mipi_dsi.h>
@@ -76,6 +77,7 @@ struct dw_mipi_dsi_stm {
 	u32 hw_version;
 	int lane_min_kbps;
 	int lane_max_kbps;
+	struct regulator *vdd_supply;
 };
 
 static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
@@ -318,17 +320,31 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
 		return PTR_ERR(dsi->base);
 	}
 
+	dsi->vdd_supply = devm_regulator_get_optional(dev, "phy-dsi");
+	if (IS_ERR(dsi->vdd_supply)) {
+		ret = PTR_ERR(dsi->vdd_supply);
+		if (ret != -EPROBE_DEFER)
+			DRM_ERROR("failed to request regulator: %d\n", ret);
+		return ret;
+	}
+
+	ret = regulator_enable(dsi->vdd_supply);
+	if (ret) {
+		DRM_ERROR("failed to enable regulator: %d\n", ret);
+		return ret;
+	}
+
 	dsi->pllref_clk = devm_clk_get(dev, "ref");
 	if (IS_ERR(dsi->pllref_clk)) {
 		ret = PTR_ERR(dsi->pllref_clk);
-		dev_err(dev, "Unable to get pll reference clock: %d\n", ret);
-		return ret;
+		DRM_ERROR("Unable to get pll reference clock: %d\n", ret);
+		goto err_clk_get;
 	}
 
 	ret = clk_prepare_enable(dsi->pllref_clk);
 	if (ret) {
-		dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);
-		return ret;
+		DRM_ERROR("%s: Failed to enable pllref_clk\n", __func__);
+		goto err_clk_get;
 	}
 
 	dw_mipi_dsi_stm_plat_data.base = dsi->base;
@@ -339,11 +355,19 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
 	dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
 	if (IS_ERR(dsi->dsi)) {
 		DRM_ERROR("Failed to initialize mipi dsi host\n");
-		clk_disable_unprepare(dsi->pllref_clk);
-		return PTR_ERR(dsi->dsi);
+		ret = PTR_ERR(dsi->dsi);
+		goto err_dsi_probe;
 	}
 
 	return 0;
+
+err_dsi_probe:
+	clk_disable_unprepare(dsi->pllref_clk);
+err_clk_get:
+	regulator_disable(dsi->vdd_supply);
+
+	return ret;
+
 }
 
 static int dw_mipi_dsi_stm_remove(struct platform_device *pdev)
@@ -351,6 +375,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device *pdev)
 	struct dw_mipi_dsi_stm *dsi = platform_get_drvdata(pdev);
 
 	clk_disable_unprepare(dsi->pllref_clk);
+	regulator_disable(dsi->vdd_supply);
 	dw_mipi_dsi_remove(dsi->dsi);
 
 	return 0;
@@ -363,6 +388,7 @@ static int __maybe_unused dw_mipi_dsi_stm_suspend(struct device *dev)
 	DRM_DEBUG_DRIVER("\n");
 
 	clk_disable_unprepare(dsi->pllref_clk);
+	regulator_disable(dsi->vdd_supply);
 
 	return 0;
 }
@@ -370,9 +396,16 @@ static int __maybe_unused dw_mipi_dsi_stm_suspend(struct device *dev)
 static int __maybe_unused dw_mipi_dsi_stm_resume(struct device *dev)
 {
 	struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
+	int ret;
 
 	DRM_DEBUG_DRIVER("\n");
 
+	ret = regulator_enable(dsi->vdd_supply);
+	if (ret) {
+		DRM_ERROR("failed to enable regulator: %d\n", ret);
+		return ret;
+	}
+
 	clk_prepare_enable(dsi->pllref_clk);
 
 	return 0;
-- 
2.7.4

[PATCH v2 3/5] ARM: dts: stm32: add phy-dsi-supply property on stm32mp157c

[Yannick Fertré]

The dsi physical layer is powered by the 1v8 power controller supply.

Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
---
 arch/arm/boot/dts/stm32mp157c.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index 2afeee6..6b14f1e 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -1156,6 +1156,7 @@
 			clock-names = "pclk", "ref", "px_clk";
 			resets = <&rcc DSI_R>;
 			reset-names = "apb";
+			phy-dsi-supply = <&reg18>;
 			status = "disabled";
 		};
 
-- 
2.7.4

[PATCH v2 4/5] ARM: dts: stm32: move fixe regulators reg11 & reg18

[Yannick Fertré]

Move regulators reg11 & reg18 from device-tree files stm32mp157c-ed1.dts
& stm32mp157c-dk2.dts to file stm32mp157c.dtsi.

Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
---
 arch/arm/boot/dts/stm32mp157c-dk2.dts |  8 --------
 arch/arm/boot/dts/stm32mp157c-ed1.dts | 16 ----------------
 arch/arm/boot/dts/stm32mp157c.dtsi    | 16 ++++++++++++++++
 3 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts
index 20ea601..020ea0f 100644
--- a/arch/arm/boot/dts/stm32mp157c-dk2.dts
+++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts
@@ -11,14 +11,6 @@
 / {
 	model = "STMicroelectronics STM32MP157C-DK2 Discovery Board";
 	compatible = "st,stm32mp157c-dk2", "st,stm32mp157";
-
-	reg18: reg18 {
-		compatible = "regulator-fixed";
-		regulator-name = "reg18";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-		regulator-always-on;
-	};
 };
 
 &dsi {
diff --git a/arch/arm/boot/dts/stm32mp157c-ed1.dts b/arch/arm/boot/dts/stm32mp157c-ed1.dts
index 62a8c78..f41189c 100644
--- a/arch/arm/boot/dts/stm32mp157c-ed1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ed1.dts
@@ -27,22 +27,6 @@
 		serial0 = &uart4;
 	};
 
-	reg11: reg11 {
-		compatible = "regulator-fixed";
-		regulator-name = "reg11";
-		regulator-min-microvolt = <1100000>;
-		regulator-max-microvolt = <1100000>;
-		regulator-always-on;
-	};
-
-	reg18: reg18 {
-		compatible = "regulator-fixed";
-		regulator-name = "reg18";
-		regulator-min-microvolt = <1800000>;
-		regulator-max-microvolt = <1800000>;
-		regulator-always-on;
-	};
-
 	sd_switch: regulator-sd_switch {
 		compatible = "regulator-gpio";
 		regulator-name = "sd_switch";
diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
index 6b14f1e..aaac51cd 100644
--- a/arch/arm/boot/dts/stm32mp157c.dtsi
+++ b/arch/arm/boot/dts/stm32mp157c.dtsi
@@ -11,6 +11,22 @@
 	#address-cells = <1>;
 	#size-cells = <1>;
 
+	reg11: reg11 {
+		compatible = "regulator-fixed";
+		regulator-name = "reg11";
+		regulator-min-microvolt = <1100000>;
+		regulator-max-microvolt = <1100000>;
+		regulator-always-on;
+	};
+
+	reg18: reg18 {
+		compatible = "regulator-fixed";
+		regulator-name = "reg18";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-always-on;
+	};
+
 	cpus {
 		#address-cells = <1>;
 		#size-cells = <0>;
-- 
2.7.4

[PATCH v2 5/5] ARM: dts: stm32: remove phy-dsi-supply property on stm32mp157c-dk2 board

[Yannick Fertré]

This property is already defined into stm32mp157c.dtsi file.

Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
---
 arch/arm/boot/dts/stm32mp157c-dk2.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts
index 020ea0f..09f6e7b 100644
--- a/arch/arm/boot/dts/stm32mp157c-dk2.dts
+++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts
@@ -17,7 +17,6 @@
 	#address-cells = <1>;
 	#size-cells = <0>;
 	status = "okay";
-	phy-dsi-supply = <&reg18>;
 
 	ports {
 		#address-cells = <1>;
-- 
2.7.4

Re: [PATCH v2 1/5] dt-bindings: display: stm32: add supply property to DSI controller

[Philippe CORNU]

Dear Yannick,
Thank you for your patch,

(already ;-)
Reviewed-by: Philippe Cornu <philippe.cornu@st.com>

Philippe :)


On 5/10/19 4:20 PM, Yannick Fertré wrote:
> This patch adds documentation of a new property phy-dsi-supply to the
> STM32 DSI controller.
> 
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
>   Documentation/devicetree/bindings/display/st,stm32-ltdc.txt | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt b/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
> index 3eb1b48..60c54da 100644
> --- a/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
> +++ b/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
> @@ -40,6 +40,8 @@ Mandatory nodes specific to STM32 DSI:
>   - panel or bridge node: A node containing the panel or bridge description as
>     documented in [6].
>     - port: panel or bridge port node, connected to the DSI output port (port@1).
> +Optional properties:
> +- phy-dsi-supply: phandle of the regulator that provides the supply voltage.
>   
>   Note: You can find more documentation in the following references
>   [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> @@ -101,6 +103,7 @@ Example 2: DSI panel
>   			clock-names = "pclk", "ref";
>   			resets = <&rcc STM32F4_APB2_RESET(DSI)>;
>   			reset-names = "apb";
> +			phy-dsi-supply = <&reg18>;
>   
>   			ports {
>   				#address-cells = <1>;
> 

Re: [PATCH v2 2/3] drm/stm: dsi: add support of an optional regulator

[Philippe CORNU]

Dear Yannick,
Thank you for your patch,

I like better the new shorter commit heading, thank you.


On 5/10/19 4:20 PM, Yannick Fertré wrote:
> Add support of an optional regulator for the phy part of the DSI
> controller.
> 
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
>   drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 45 ++++++++++++++++++++++++++++++-----
>   1 file changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> index 1bef73e..22bd095 100644
> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> @@ -9,6 +9,7 @@
>   #include <linux/clk.h>
>   #include <linux/iopoll.h>
>   #include <linux/module.h>
> +#include <linux/regulator/consumer.h>
>   #include <drm/drmP.h>
>   #include <drm/drm_mipi_dsi.h>
>   #include <drm/bridge/dw_mipi_dsi.h>
> @@ -76,6 +77,7 @@ struct dw_mipi_dsi_stm {
>   	u32 hw_version;
>   	int lane_min_kbps;
>   	int lane_max_kbps;
> +	struct regulator *vdd_supply;
>   };
>   
>   static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
> @@ -318,17 +320,31 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
>   		return PTR_ERR(dsi->base);
>   	}
>   
> +	dsi->vdd_supply = devm_regulator_get_optional(dev, "phy-dsi");
> +	if (IS_ERR(dsi->vdd_supply)) {
> +		ret = PTR_ERR(dsi->vdd_supply);
> +		if (ret != -EPROBE_DEFER)
> +			DRM_ERROR("failed to request regulator: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = regulator_enable(dsi->vdd_supply);
> +	if (ret) {
> +		DRM_ERROR("failed to enable regulator: %d\n", ret);
> +		return ret;
> +	}
> +
>   	dsi->pllref_clk = devm_clk_get(dev, "ref");
>   	if (IS_ERR(dsi->pllref_clk)) {
>   		ret = PTR_ERR(dsi->pllref_clk);
> -		dev_err(dev, "Unable to get pll reference clock: %d\n", ret);
> -		return ret;
> +		DRM_ERROR("Unable to get pll reference clock: %d\n", ret);
> +		goto err_clk_get;
>   	}
>   
>   	ret = clk_prepare_enable(dsi->pllref_clk);
>   	if (ret) {
> -		dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);
> -		return ret;
> +		DRM_ERROR("%s: Failed to enable pllref_clk\n", __func__);
> +		goto err_clk_get;
>   	}
>   
>   	dw_mipi_dsi_stm_plat_data.base = dsi->base;
> @@ -339,11 +355,19 @@ static int dw_mipi_dsi_stm_probe(struct platform_device *pdev)
>   	dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
>   	if (IS_ERR(dsi->dsi)) {
>   		DRM_ERROR("Failed to initialize mipi dsi host\n");
> -		clk_disable_unprepare(dsi->pllref_clk);
> -		return PTR_ERR(dsi->dsi);
> +		ret = PTR_ERR(dsi->dsi);
> +		goto err_dsi_probe;
>   	}
>   
>   	return 0;
> +
> +err_dsi_probe:
> +	clk_disable_unprepare(dsi->pllref_clk);
> +err_clk_get:
> +	regulator_disable(dsi->vdd_supply);
> +
> +	return ret;
> +
>   }
>   
>   static int dw_mipi_dsi_stm_remove(struct platform_device *pdev)
> @@ -351,6 +375,7 @@ static int dw_mipi_dsi_stm_remove(struct platform_device *pdev)
>   	struct dw_mipi_dsi_stm *dsi = platform_get_drvdata(pdev);
>   
>   	clk_disable_unprepare(dsi->pllref_clk);
> +	regulator_disable(dsi->vdd_supply);
>   	dw_mipi_dsi_remove(dsi->dsi);

for a future patch: we may have a different order
	dw_mipi_dsi_remove(dsi->dsi);   	
	clk_disable_unprepare(dsi->pllref_clk);
	regulator_disable(dsi->vdd_supply);

>   
>   	return 0;
> @@ -363,6 +388,7 @@ static int __maybe_unused dw_mipi_dsi_stm_suspend(struct device *dev)
>   	DRM_DEBUG_DRIVER("\n");
>   
>   	clk_disable_unprepare(dsi->pllref_clk);
> +	regulator_disable(dsi->vdd_supply);
>   
>   	return 0;
>   }
> @@ -370,9 +396,16 @@ static int __maybe_unused dw_mipi_dsi_stm_suspend(struct device *dev)
>   static int __maybe_unudw_mipi_dsi_remove(dsi->dsi);sed dw_mipi_dsi_stm_resume(struct device *dev)
>   {
>   	struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
> +	int ret;
>   
>   	DRM_DEBUG_DRIVER("\n");
>   
> +	ret = regulator_enable(dsi->vdd_supply);
> +	if (ret) {
> +		DRM_ERROR("failed to enable regulator: %d\n", ret);
> +		return ret;
> +	}
> +
>   	clk_prepare_enable(dsi->pllref_clk);

for a future patch: we may check clk_prepare_enable return value.

>   
>   	return 0;
> 


Acked-by: Philippe Cornu <philippe.cornu@st.com>

Philippe :)

Re: [PATCH v2 3/5] ARM: dts: stm32: add phy-dsi-supply property on stm32mp157c

[Philippe CORNU]

Dear Yannick,
Thank you for your patch,

Reviewed-by: Philippe Cornu <philippe.cornu@st.com>

Philippe :-)

On 5/10/19 4:20 PM, Yannick Fertré wrote:
> The dsi physical layer is powered by the 1v8 power controller supply.
> 
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
>   arch/arm/boot/dts/stm32mp157c.dtsi | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
> index 2afeee6..6b14f1e 100644
> --- a/arch/arm/boot/dts/stm32mp157c.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c.dtsi
> @@ -1156,6 +1156,7 @@
>   			clock-names = "pclk", "ref", "px_clk";
>   			resets = <&rcc DSI_R>;
>   			reset-names = "apb";
> +			phy-dsi-supply = <&reg18>;
>   			status = "disabled";
>   		};
>   
> 

Re: [PATCH v2 2/3] drm/stm: dsi: add support of an optional regulator

[Philippe CORNU]

Hi Yannick,


On 5/10/19 6:16 PM, Philippe Cornu wrote:
> Dear Yannick,
> Thank you for your patch,
> 
> I like better the new shorter commit heading, thank you.
> 
> 
> On 5/10/19 4:20 PM, Yannick Fertré wrote:
>> Add support of an optional regulator for the phy part of the DSI
>> controller.
>>
>> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
>> ---
>>   drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 45 
>> ++++++++++++++++++++++++++++++-----
>>   1 file changed, 39 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c 
>> b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
>> index 1bef73e..22bd095 100644
>> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
>> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
>> @@ -9,6 +9,7 @@
>>   #include <linux/clk.h>
>>   #include <linux/iopoll.h>
>>   #include <linux/module.h>
>> +#include <linux/regulator/consumer.h>
>>   #include <drm/drmP.h>
>>   #include <drm/drm_mipi_dsi.h>
>>   #include <drm/bridge/dw_mipi_dsi.h>
>> @@ -76,6 +77,7 @@ struct dw_mipi_dsi_stm {
>>       u32 hw_version;
>>       int lane_min_kbps;
>>       int lane_max_kbps;
>> +    struct regulator *vdd_supply;
>>   };
>>   static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, 
>> u32 val)
>> @@ -318,17 +320,31 @@ static int dw_mipi_dsi_stm_probe(struct 
>> platform_device *pdev)
>>           return PTR_ERR(dsi->base);
>>       }
>> +    dsi->vdd_supply = devm_regulator_get_optional(dev, "phy-dsi");
>> +    if (IS_ERR(dsi->vdd_supply)) {
>> +        ret = PTR_ERR(dsi->vdd_supply);
>> +        if (ret != -EPROBE_DEFER)
>> +            DRM_ERROR("failed to request regulator: %d\n", ret);
>> +        return ret;
>> +    }


I did more tests on a stm32f469 disco board and this above code does not 
work (the ret value is -ENODEV)

Two possibilities then:
1) remove the _optional (but we have the "using dummy regulator" 
message, I am fine with it)

or
2) handle -ENODEV but do not forget to check vdd_supply!=NULL before 
calling all regulator_(enable/disable)

	if (ret != -ENODEV) {
		if (ret != -EPROBE_DEFER)
			DRM_ERROR("failed to request regulator: %d\n",
				ret);
			return ret;
		}
		dsi->vdd_supply = NULL;
	}

	if (dsi->vdd_supply) {
		ret = regulator_enable(dsi->vdd_supply);
		...


	if (dsi->vdd_supply)
		regulator_disable(dsi->vdd_supply);


I let you choose your favorite one.

Thank you,
Philippe

>> +
>> +    ret = regulator_enable(dsi->vdd_supply);
>> +    if (ret) {
>> +        DRM_ERROR("failed to enable regulator: %d\n", ret);
>> +        return ret;
>> +    }
>> +
>>       dsi->pllref_clk = devm_clk_get(dev, "ref");
>>       if (IS_ERR(dsi->pllref_clk)) {
>>           ret = PTR_ERR(dsi->pllref_clk);
>> -        dev_err(dev, "Unable to get pll reference clock: %d\n", ret);
>> -        return ret;
>> +        DRM_ERROR("Unable to get pll reference clock: %d\n", ret);
>> +        goto err_clk_get;
>>       }
>>       ret = clk_prepare_enable(dsi->pllref_clk);
>>       if (ret) {
>> -        dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);
>> -        return ret;
>> +        DRM_ERROR("%s: Failed to enable pllref_clk\n", __func__);
>> +        goto err_clk_get;
>>       }
>>       dw_mipi_dsi_stm_plat_data.base = dsi->base;
>> @@ -339,11 +355,19 @@ static int dw_mipi_dsi_stm_probe(struct 
>> platform_device *pdev)
>>       dsi->dsi = dw_mipi_dsi_probe(pdev, &dw_mipi_dsi_stm_plat_data);
>>       if (IS_ERR(dsi->dsi)) {
>>           DRM_ERROR("Failed to initialize mipi dsi host\n");
>> -        clk_disable_unprepare(dsi->pllref_clk);
>> -        return PTR_ERR(dsi->dsi);
>> +        ret = PTR_ERR(dsi->dsi);
>> +        goto err_dsi_probe;
>>       }
>>       return 0;
>> +
>> +err_dsi_probe:
>> +    clk_disable_unprepare(dsi->pllref_clk);
>> +err_clk_get:
>> +    regulator_disable(dsi->vdd_supply);
>> +
>> +    return ret;
>> +
>>   }
>>   static int dw_mipi_dsi_stm_remove(struct platform_device *pdev)
>> @@ -351,6 +375,7 @@ static int dw_mipi_dsi_stm_remove(struct 
>> platform_device *pdev)
>>       struct dw_mipi_dsi_stm *dsi = platform_get_drvdata(pdev);
>>       clk_disable_unprepare(dsi->pllref_clk);
>> +    regulator_disable(dsi->vdd_supply);
>>       dw_mipi_dsi_remove(dsi->dsi);
> 
> for a future patch: we may have a different order
>      dw_mipi_dsi_remove(dsi->dsi);
>      clk_disable_unprepare(dsi->pllref_clk);
>      regulator_disable(dsi->vdd_supply);
> 
>>       return 0;
>> @@ -363,6 +388,7 @@ static int __maybe_unused 
>> dw_mipi_dsi_stm_suspend(struct device *dev)
>>       DRM_DEBUG_DRIVER("\n");
>>       clk_disable_unprepare(dsi->pllref_clk);
>> +    regulator_disable(dsi->vdd_supply);
>>       return 0;
>>   }
>> @@ -370,9 +396,16 @@ static int __maybe_unused 
>> dw_mipi_dsi_stm_suspend(struct device *dev)
>>   static int __maybe_unudw_mipi_dsi_remove(dsi->dsi);sed 
>> dw_mipi_dsi_stm_resume(struct device *dev)
>>   {
>>       struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
>> +    int ret;
>>       DRM_DEBUG_DRIVER("\n");
>> +    ret = regulator_enable(dsi->vdd_supply);
>> +    if (ret) {
>> +        DRM_ERROR("failed to enable regulator: %d\n", ret);
>> +        return ret;
>> +    }
>> +
>>       clk_prepare_enable(dsi->pllref_clk);
> 
> for a future patch: we may check clk_prepare_enable return value.
> 
>>       return 0;
>>
> 
> 
> Acked-by: Philippe Cornu <philippe.cornu@st.com>
> 
> Philippe :)
> 

Re: [PATCH v2 1/5] dt-bindings: display: stm32: add supply property to DSI controller

[Rob Herring]

On Fri, 10 May 2019 16:20:19 +0200, =?UTF-8?q?Yannick=20Fertr=C3=A9?= wrote:
> This patch adds documentation of a new property phy-dsi-supply to the
> STM32 DSI controller.
> 
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
>  Documentation/devicetree/bindings/display/st,stm32-ltdc.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v2 3/5] ARM: dts: stm32: add phy-dsi-supply property on stm32mp157c

[Yannick FERTRE]

Hi Alexandre,

this patch can be abandoned.

BR

-- 
Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270
Microcontrollers and Digital ICs Group | Microcontrolleurs Division


On 5/10/19 4:20 PM, Yannick Fertré wrote:

> The dsi physical layer is powered by the 1v8 power controller supply.
>
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
>   arch/arm/boot/dts/stm32mp157c.dtsi | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
> index 2afeee6..6b14f1e 100644
> --- a/arch/arm/boot/dts/stm32mp157c.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c.dtsi
> @@ -1156,6 +1156,7 @@
>   			clock-names = "pclk", "ref", "px_clk";
>   			resets = <&rcc DSI_R>;
>   			reset-names = "apb";
> +			phy-dsi-supply = <&reg18>;
>   			status = "disabled";
>   		};
>   

Re: [PATCH v2 4/5] ARM: dts: stm32: move fixe regulators reg11 & reg18

[Yannick FERTRE]

Hi Alexandre,

this patch can be abandoned.

BR

-- 
Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270
Microcontrollers and Digital ICs Group | Microcontrolleurs Division

On 5/10/19 4:20 PM, Yannick Fertré wrote:
> Move regulators reg11 & reg18 from device-tree files stm32mp157c-ed1.dts
> & stm32mp157c-dk2.dts to file stm32mp157c.dtsi.
>
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
>   arch/arm/boot/dts/stm32mp157c-dk2.dts |  8 --------
>   arch/arm/boot/dts/stm32mp157c-ed1.dts | 16 ----------------
>   arch/arm/boot/dts/stm32mp157c.dtsi    | 16 ++++++++++++++++
>   3 files changed, 16 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> index 20ea601..020ea0f 100644
> --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> @@ -11,14 +11,6 @@
>   / {
>   	model = "STMicroelectronics STM32MP157C-DK2 Discovery Board";
>   	compatible = "st,stm32mp157c-dk2", "st,stm32mp157";
> -
> -	reg18: reg18 {
> -		compatible = "regulator-fixed";
> -		regulator-name = "reg18";
> -		regulator-min-microvolt = <1800000>;
> -		regulator-max-microvolt = <1800000>;
> -		regulator-always-on;
> -	};
>   };
>   
>   &dsi {
> diff --git a/arch/arm/boot/dts/stm32mp157c-ed1.dts b/arch/arm/boot/dts/stm32mp157c-ed1.dts
> index 62a8c78..f41189c 100644
> --- a/arch/arm/boot/dts/stm32mp157c-ed1.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-ed1.dts
> @@ -27,22 +27,6 @@
>   		serial0 = &uart4;
>   	};
>   
> -	reg11: reg11 {
> -		compatible = "regulator-fixed";
> -		regulator-name = "reg11";
> -		regulator-min-microvolt = <1100000>;
> -		regulator-max-microvolt = <1100000>;
> -		regulator-always-on;
> -	};
> -
> -	reg18: reg18 {
> -		compatible = "regulator-fixed";
> -		regulator-name = "reg18";
> -		regulator-min-microvolt = <1800000>;
> -		regulator-max-microvolt = <1800000>;
> -		regulator-always-on;
> -	};
> -
>   	sd_switch: regulator-sd_switch {
>   		compatible = "regulator-gpio";
>   		regulator-name = "sd_switch";
> diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi
> index 6b14f1e..aaac51cd 100644
> --- a/arch/arm/boot/dts/stm32mp157c.dtsi
> +++ b/arch/arm/boot/dts/stm32mp157c.dtsi
> @@ -11,6 +11,22 @@
>   	#address-cells = <1>;
>   	#size-cells = <1>;
>   
> +	reg11: reg11 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "reg11";
> +		regulator-min-microvolt = <1100000>;
> +		regulator-max-microvolt = <1100000>;
> +		regulator-always-on;
> +	};
> +
> +	reg18: reg18 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "reg18";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		regulator-always-on;
> +	};
> +
>   	cpus {
>   		#address-cells = <1>;
>   		#size-cells = <0>;

Re: [PATCH v2 5/5] ARM: dts: stm32: remove phy-dsi-supply property on stm32mp157c-dk2 board

[Yannick FERTRE]

Hi Alexandre,

this patch can be abandoned.

BR

-- 
Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270
Microcontrollers and Digital ICs Group | Microcontrolleurs Division
On 5/10/19 4:20 PM, Yannick Fertré wrote:
> This property is already defined into stm32mp157c.dtsi file.
>
> Signed-off-by: Yannick Fertré <yannick.fertre@st.com>
> ---
>   arch/arm/boot/dts/stm32mp157c-dk2.dts | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> index 020ea0f..09f6e7b 100644
> --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts
> +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts
> @@ -17,7 +17,6 @@
>   	#address-cells = <1>;
>   	#size-cells = <0>;
>   	status = "okay";
> -	phy-dsi-supply = <&reg18>;
>   
>   	ports {
>   		#address-cells = <1>;