[PATCH 0/4] drm/exynos: use syscon framework to enable mipi phy

[PATCH 0/4] drm/exynos: use syscon framework to enable mipi phy

[Inki Dae]

This patch series makes syscon framework to be used
instead of phy framework.

For this, I adds syscon support to mipi dsi driver and
the relevant device tree properties to each dtsi files,
Exynos4, Exynos3250 and Exynos5420.

Inki Dae (4):
  drm/exynos: dsim: fix to control mipi phy register
  ARM: dts: exynos4: use pmureg device node to enable mipi phy
  ARM: dts: exynos3250: use pmureg device node to enable mipi phy
  ARM: dts: exynos5420: use pmureg device node to enable mipi phy

 .../devicetree/bindings/video/exynos_dsim.txt      |    9 ++--
 arch/arm/boot/dts/exynos3250.dtsi                  |    3 +-
 arch/arm/boot/dts/exynos4.dtsi                     |    3 +-
 arch/arm/boot/dts/exynos5420.dtsi                  |    3 +-
 drivers/gpu/drm/exynos/exynos_drm_dsi.c            |   54 ++++++++++++++++++--
 5 files changed, 59 insertions(+), 13 deletions(-)

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] drm/exynos: dsim: fix to control mipi phy register

[Inki Dae]

This patch fixes the issue that the try to get a phy object is failed
to enable mipi phy.

System and power management unit registers should be controlled by
syscon framework. So this patch removes existing phy framework based
codes and adds syscon support instead. However, we should support
legacy device tree binding so consider the legacy binding for compatibility.

In addition, we need to remove below device node and relevant properties,
	mipi_phy: video-phy@10020710 {
		compatible = "samsung,s5pv210-mipi-video-phy";
		reg = <0x10020710 8>;
		#phy-cells = <1>;
	};

Now camera device node uses mipi_phy node relevant properties like below,
	camera {
		...
		csis_0: csis@11880000 {
			...
			phys = <&mipi_phy 0>;
			phy-names = "csis";
			...
		};
		csis_1: csis@11890000 {
			...
			phys = <&mipi_phy 2>;
			phy-names = "csis";
			...
		};
		...
	};

With above, we will find below message while booting,
     can't request region for resource [mem 0x10020710-0x10020717]

Signed-off-by: Inki Dae <inki.dae@samsung.com>
---
 .../devicetree/bindings/video/exynos_dsim.txt      |    9 ++--
 drivers/gpu/drm/exynos/exynos_drm_dsi.c            |   54 ++++++++++++++++++--
 2 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/video/exynos_dsim.txt b/Documentation/devicetree/bindings/video/exynos_dsim.txt
index ca2b4aa..dec3b55 100644
--- a/Documentation/devicetree/bindings/video/exynos_dsim.txt
+++ b/Documentation/devicetree/bindings/video/exynos_dsim.txt
@@ -11,15 +11,18 @@ Required properties:
   - clocks: list of clock specifiers, must contain an entry for each required
     entry in clock-names
   - clock-names: should include "bus_clk"and "pll_clk" entries
-  - phys: list of phy specifiers, must contain an entry for each required
-    entry in phy-names
-  - phy-names: should include "dsim" entry
+  - samsung,pmureg: handle to syscon used to control the PMU registers
   - vddcore-supply: MIPI DSIM Core voltage supply (e.g. 1.1V)
   - vddio-supply: MIPI DSIM I/O and PLL voltage supply (e.g. 1.8V)
   - samsung,pll-clock-frequency: specifies frequency of the "pll_clk" clock
   - #address-cells, #size-cells: should be set respectively to <1> and <0>
     according to DSI host bindings (see MIPI DSI bindings [1])
 
+Deprecated properties for MIPI DSI Master:
+  - phys: list of phy specifiers, must contain an entry for each required
+    entry in phy-names (Deprecated)
+  - phy-names: should include "dsim" entry (Deprecated)
+
 Optional properties:
   - samsung,power-domain: a phandle to DSIM power domain node
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 05fe93d..38b025e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -23,6 +23,8 @@
 #include <linux/phy/phy.h>
 #include <linux/regulator/consumer.h>
 #include <linux/component.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
 
 #include <video/mipi_display.h>
 #include <video/videomode.h>
@@ -263,6 +265,8 @@ struct exynos_dsi_transfer {
 struct exynos_dsi_driver_data {
 	unsigned int plltmr_reg;
 
+	unsigned int mipi_phy_offset;
+	unsigned int mipi_phy_en_shift;
 	unsigned int has_freqband:1;
 	unsigned int has_clklane_stop:1;
 };
@@ -277,6 +281,7 @@ struct exynos_dsi {
 
 	void __iomem *reg_base;
 	struct phy *phy;
+	struct regmap *pmureg;
 	struct clk *pll_clk;
 	struct clk *bus_clk;
 	struct regulator_bulk_data supplies[2];
@@ -313,21 +318,29 @@ static struct exynos_dsi_driver_data exynos3_dsi_driver_data = {
 	.plltmr_reg = 0x50,
 	.has_freqband = 1,
 	.has_clklane_stop = 1,
+	.mipi_phy_offset = 0x710,
+	.mipi_phy_en_shift = 0,
 };
 
 static struct exynos_dsi_driver_data exynos4_dsi_driver_data = {
 	.plltmr_reg = 0x50,
 	.has_freqband = 1,
 	.has_clklane_stop = 1,
+	.mipi_phy_offset = 0x710,
+	.mipi_phy_en_shift = 0,
 };
 
 static struct exynos_dsi_driver_data exynos4415_dsi_driver_data = {
 	.plltmr_reg = 0x58,
 	.has_clklane_stop = 1,
+	.mipi_phy_offset = 0x710,
+	.mipi_phy_en_shift = 0,
 };
 
 static struct exynos_dsi_driver_data exynos5_dsi_driver_data = {
 	.plltmr_reg = 0x58,
+	.mipi_phy_offset = 0x714,
+	.mipi_phy_en_shift = 0,
 };
 
 static struct of_device_id exynos_dsi_of_match[] = {
@@ -1294,6 +1307,7 @@ static const struct mipi_dsi_host_ops exynos_dsi_ops = {
 
 static int exynos_dsi_poweron(struct exynos_dsi *dsi)
 {
+	struct exynos_dsi_driver_data *driver_data = dsi->driver_data;
 	int ret;
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(dsi->supplies), dsi->supplies);
@@ -1314,7 +1328,14 @@ static int exynos_dsi_poweron(struct exynos_dsi *dsi)
 		goto err_pll_clk;
 	}
 
-	ret = phy_power_on(dsi->phy);
+	if (dsi->phy) {
+		ret = phy_power_on(dsi->phy);
+	} else {
+		ret = regmap_update_bits(dsi->pmureg,
+					 driver_data->mipi_phy_offset,
+					 0x1 << driver_data->mipi_phy_en_shift,
+					 0x1 << driver_data->mipi_phy_en_shift);
+	}
 	if (ret < 0) {
 		dev_err(dsi->dev, "cannot enable phy %d\n", ret);
 		goto err_phy;
@@ -1334,6 +1355,7 @@ err_bus_clk:
 
 static void exynos_dsi_poweroff(struct exynos_dsi *dsi)
 {
+	struct exynos_dsi_driver_data *driver_data = dsi->driver_data;
 	int ret;
 
 	usleep_range(10000, 20000);
@@ -1348,7 +1370,14 @@ static void exynos_dsi_poweroff(struct exynos_dsi *dsi)
 
 	dsi->state &= ~DSIM_STATE_CMD_LPM;
 
-	phy_power_off(dsi->phy);
+	if (dsi->phy) {
+		phy_power_off(dsi->phy);
+	} else {
+		regmap_update_bits(dsi->pmureg,
+				   driver_data->mipi_phy_offset,
+				   0x0 << driver_data->mipi_phy_en_shift,
+				   0x0 << driver_data->mipi_phy_en_shift);
+	}
 
 	clk_disable_unprepare(dsi->pll_clk);
 	clk_disable_unprepare(dsi->bus_clk);
@@ -1742,13 +1771,30 @@ static int exynos_dsi_probe(struct platform_device *pdev)
 		goto err_del_component;
 	}
 
+	/*
+	 * Consider legacy device tree binding.
+	 * phy framework isn't used anymore. (Deprecated)
+	 */
 	dsi->phy = devm_phy_get(dev, "dsim");
-	if (IS_ERR(dsi->phy)) {
-		dev_info(dev, "failed to get dsim phy\n");
+	if (!IS_ERR(dsi->phy))
+		goto skip_syscon;
+
+	if (PTR_ERR(dsi->phy) == -EPROBE_DEFER) {
 		ret = PTR_ERR(dsi->phy);
 		goto err_del_component;
 	}
 
+	dsi->phy = NULL;
+
+	dsi->pmureg = syscon_regmap_lookup_by_phandle(dev->of_node,
+							"samsung,pmureg");
+	if (IS_ERR(dsi->pmureg)) {
+		dev_warn(dev, "failed to get pmu register.\n");
+		ret = PTR_ERR(dsi->pmureg);
+		goto err_del_component;
+	}
+
+skip_syscon:
 	dsi->irq = platform_get_irq(pdev, 0);
 	if (dsi->irq < 0) {
 		dev_err(dev, "failed to request dsi irq resource\n");
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] ARM: dts: exynos4: use pmureg device node to enable mipi phy

[Inki Dae]

This patch removes mipi phy relevant properties from dsim device node
and makes the pmureg device node to be used instead to enable
mipi phy.

Signed-off-by: Inki Dae <inki.dae@samsung.com>
---
 arch/arm/boot/dts/exynos4.dtsi |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index b8168f1..42dfdcf 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -148,8 +148,7 @@
 		reg = <0x11C80000 0x10000>;
 		interrupts = <0 79 0>;
 		samsung,power-domain = <&pd_lcd0>;
-		phys = <&mipi_phy 1>;
-		phy-names = "dsim";
+		samsung,pmureg = <&pmu_system_controller>;
 		clocks = <&clock CLK_DSIM0>, <&clock CLK_SCLK_MIPI0>;
 		clock-names = "bus_clk", "pll_clk";
 		status = "disabled";
-- 
1.7.9.5

[PATCH 3/4] ARM: dts: exynos3250: use pmureg device node to enable mipi phy

[Inki Dae]

This patch removes mipi phy relevant properties from dsim device node
and makes the pmureg device node to be used instead to enable
mipi phy.

Signed-off-by: Inki Dae <inki.dae@samsung.com>
---
 arch/arm/boot/dts/exynos3250.dtsi |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
index 2246549..eb80802 100644
--- a/arch/arm/boot/dts/exynos3250.dtsi
+++ b/arch/arm/boot/dts/exynos3250.dtsi
@@ -246,8 +246,7 @@
 			interrupts = <0 83 0>;
 			samsung,phy-type = <0>;
 			samsung,power-domain = <&pd_lcd0>;
-			phys = <&mipi_phy 1>;
-			phy-names = "dsim";
+			samsung,pmureg = <&pmu_system_controller>;
 			clocks = <&cmu CLK_DSIM0>, <&cmu CLK_SCLK_MIPI0>;
 			clock-names = "bus_clk", "pll_clk";
 			#address-cells = <1>;
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] ARM: dts: exynos5420: use pmureg device node to enable mipi phy

[Inki Dae]

This patch removes mipi phy relevant properties from dsim device node
and makes the pmureg device node to be used instead to enable
mipi phy.

Signed-off-by: Inki Dae <inki.dae@samsung.com>
---
 arch/arm/boot/dts/exynos5420.dtsi |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 6d38f8b..b8b8826 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -525,8 +525,7 @@
 		compatible = "samsung,exynos5410-mipi-dsi";
 		reg = <0x14500000 0x10000>;
 		interrupts = <0 82 0>;
-		phys = <&mipi_phy 1>;
-		phy-names = "dsim";
+		samsung,pmureg = <&pmu_system_controller>;
 		clocks = <&clock CLK_DSIM1>, <&clock CLK_SCLK_MIPI1>;
 		clock-names = "bus_clk", "pll_clk";
 		#address-cells = <1>;
-- 
1.7.9.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/exynos: dsim: fix to control mipi phy register

[Sylwester Nawrocki]

On 07/02/15 12:53, Inki Dae wrote:
> This patch fixes the issue that the try to get a phy object is failed
> to enable mipi phy.
> 
> System and power management unit registers should be controlled by
> syscon framework. So this patch removes existing phy framework based
> codes and adds syscon support instead. However, we should support
> legacy device tree binding so consider the legacy binding for compatibility.
> 
> In addition, we need to remove below device node and relevant properties,
> 	mipi_phy: video-phy@10020710 {
> 		compatible = "samsung,s5pv210-mipi-video-phy";
> 		reg = <0x10020710 8>;
> 		#phy-cells = <1>;
> 	};
> 
> Now camera device node uses mipi_phy node relevant properties like below,
> 	camera {
> 		...
> 		csis_0: csis@11880000 {
> 			...
> 			phys = <&mipi_phy 0>;
> 			phy-names = "csis";
> 			...
> 		};
> 		csis_1: csis@11890000 {
> 			...
> 			phys = <&mipi_phy 2>;
> 			phy-names = "csis";
> 			...
> 		};
> 		...
> 	};
> 
> With above, we will find below message while booting,
>      can't request region for resource [mem 0x10020710-0x10020717]

I'm afraid this approach won't work because MIPI DSI Master and MIPI CSI
Slave devices share a control bit in the register and it seems impossible
to ensure proper locking with current regmap/syscon API.

I have submitted patches to fix this issue [1] and they should be already
available in linux-next and can be found on linux-samsung-soc ML:

[PATCH 1/2] phy: exynos-video-mipi: Fix regression by adding support for PMU regmap
[PATCH 2/2] ARM: dts: Add syscon phandle to the video-phy node for Exynos4

The other issue with your approach is that we are moving the PMU details
to the MIPI DSIM driver and similar changes would need to be done in
the MIPI CSIS driver.

Instead I just added syscon support to the PHY layer, it's not perfect
but fixes the issue for both DSI and CSI and  doesn't strip the PHY layer
which could potentially be useful.

-- 
Thanks,
Sylwester

[1] https://patchwork.ozlabs.org/patch/429948/
    http://www.spinics.net/lists/linux-samsung-soc/msg41210.html

Re: [PATCH 1/4] drm/exynos: dsim: fix to control mipi phy register

[Inki Dae]

On 2015년 02월 09일 19:57, Sylwester Nawrocki wrote:
> On 07/02/15 12:53, Inki Dae wrote:
>> This patch fixes the issue that the try to get a phy object is failed
>> to enable mipi phy.
>>
>> System and power management unit registers should be controlled by
>> syscon framework. So this patch removes existing phy framework based
>> codes and adds syscon support instead. However, we should support
>> legacy device tree binding so consider the legacy binding for compatibility.
>>
>> In addition, we need to remove below device node and relevant properties,
>> 	mipi_phy: video-phy@10020710 {
>> 		compatible = "samsung,s5pv210-mipi-video-phy";
>> 		reg = <0x10020710 8>;
>> 		#phy-cells = <1>;
>> 	};
>>
>> Now camera device node uses mipi_phy node relevant properties like below,
>> 	camera {
>> 		...
>> 		csis_0: csis@11880000 {
>> 			...
>> 			phys = <&mipi_phy 0>;
>> 			phy-names = "csis";
>> 			...
>> 		};
>> 		csis_1: csis@11890000 {
>> 			...
>> 			phys = <&mipi_phy 2>;
>> 			phy-names = "csis";
>> 			...
>> 		};
>> 		...
>> 	};
>>
>> With above, we will find below message while booting,
>>      can't request region for resource [mem 0x10020710-0x10020717]
> 
> I'm afraid this approach won't work because MIPI DSI Master and MIPI CSI
> Slave devices share a control bit in the register and it seems impossible
> to ensure proper locking with current regmap/syscon API.
> 
> I have submitted patches to fix this issue [1] and they should be already
> available in linux-next and can be found on linux-samsung-soc ML:
> 
> [PATCH 1/2] phy: exynos-video-mipi: Fix regression by adding support for PMU regmap
> [PATCH 2/2] ARM: dts: Add syscon phandle to the video-phy node for Exynos4
> 
> The other issue with your approach is that we are moving the PMU details
> to the MIPI DSIM driver and similar changes would need to be done in
> the MIPI CSIS driver.
> 
> Instead I just added syscon support to the PHY layer, it's not perfect
> but fixes the issue for both DSI and CSI and  doesn't strip the PHY layer
> which could potentially be useful.

Ah, Right. I didn't check your patch set. Your way is a better idea than
my one. With this, we don't need to change device drivers, MIPI DSI and CSI.

Then, what is the meaning that it's not perfect?

Thanks,
Inki Dae

> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/4] drm/exynos: dsim: fix to control mipi phy register

[Sylwester Nawrocki]

On 09/02/15 13:17, Inki Dae wrote:
>> Instead I just added syscon support to the PHY layer, it's not perfect
>> > but fixes the issue for both DSI and CSI and  doesn't strip the PHY layer
>> > which could potentially be useful.
>
> Ah, Right. I didn't check your patch set. Your way is a better idea than
> my one. With this, we don't need to change device drivers, MIPI DSI and CSI.
> 
> Then, what is the meaning that it's not perfect?

What I didn't like is that there are at least 3 mutexes on the phy_power_on/
phy_power_off path (PHY core, PHY driver, regmap) and there is another level
of indirection after introducing the regmap. I guess it's nothing serious
though. And BTW the syscon could be converted to use spinlock rather than
a mutex, since in our case behind the PMU syscon is always a memory mapped
region.

-- 
Regards,
Sylwester