Now that we have moved to generic phy based bindings, we don't need to have any code related to older dptx-phy. Nobody is using this dptx-phy anymore, so removing the same.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com Cc: Inki Dae inki.dae@samsung.com Cc: Jingoo Han jg1.han@samsung.com ---
Changes from V1: - Reworked error handling in exynos_dp_dt_parse_phydata() as commented by Inki.
drivers/gpu/drm/exynos/exynos_dp_core.c | 67 ++++++++----------------------- drivers/gpu/drm/exynos/exynos_dp_core.h | 2 - 2 files changed, 17 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index cd50ece..206163b 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -1052,28 +1052,14 @@ static int exynos_dp_create_connector(struct exynos_drm_display *display,
static void exynos_dp_phy_init(struct exynos_dp_device *dp) { - if (dp->phy) { + if (dp->phy) phy_power_on(dp->phy); - } else if (dp->phy_addr) { - u32 reg; - - reg = __raw_readl(dp->phy_addr); - reg |= dp->enable_mask; - __raw_writel(reg, dp->phy_addr); - } }
static void exynos_dp_phy_exit(struct exynos_dp_device *dp) { - if (dp->phy) { + if (dp->phy) phy_power_off(dp->phy); - } else if (dp->phy_addr) { - u32 reg; - - reg = __raw_readl(dp->phy_addr); - reg &= ~(dp->enable_mask); - __raw_writel(reg, dp->phy_addr); - } }
static void exynos_dp_poweron(struct exynos_drm_display *display) @@ -1212,40 +1198,13 @@ static struct video_info *exynos_dp_dt_parse_pdata(struct device *dev)
static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp) { - struct device_node *dp_phy_node = of_node_get(dp->dev->of_node); - u32 phy_base; - int ret = 0; - - dp_phy_node = of_find_node_by_name(dp_phy_node, "dptx-phy"); - if (!dp_phy_node) { - dp->phy = devm_phy_get(dp->dev, "dp"); - return PTR_ERR_OR_ZERO(dp->phy); - } - - if (of_property_read_u32(dp_phy_node, "reg", &phy_base)) { - dev_err(dp->dev, "failed to get reg for dptx-phy\n"); - ret = -EINVAL; - goto err; - } - - if (of_property_read_u32(dp_phy_node, "samsung,enable-mask", - &dp->enable_mask)) { - dev_err(dp->dev, "failed to get enable-mask for dptx-phy\n"); - ret = -EINVAL; - goto err; - } - - dp->phy_addr = ioremap(phy_base, SZ_4); - if (!dp->phy_addr) { - dev_err(dp->dev, "failed to ioremap dp-phy\n"); - ret = -ENOMEM; - goto err; + dp->phy = devm_phy_get(dp->dev, "dp"); + if (IS_ERR(dp->phy)) { + dev_err(dp->dev, "no DP phy configured\n"); + return PTR_ERR(dp->phy); }
-err: - of_node_put(dp_phy_node); - - return ret; + return 0; }
static int exynos_dp_dt_parse_panel(struct exynos_dp_device *dp) @@ -1278,8 +1237,16 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) return PTR_ERR(dp->video_info);
ret = exynos_dp_dt_parse_phydata(dp); - if (ret) - return ret; + if (ret) { + /* + * phy itself is not enabled, so we can move forward + * assigning NULL to phy pointer. + */ + if (ret == -ENOSYS || ret == -ENODEV) + dp->phy = NULL; + else + return ret; + }
if (!dp->panel) { ret = exynos_dp_dt_parse_panel(dp); diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.h b/drivers/gpu/drm/exynos/exynos_dp_core.h index a1aee69..6426201 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.h +++ b/drivers/gpu/drm/exynos/exynos_dp_core.h @@ -153,8 +153,6 @@ struct exynos_dp_device { struct clk *clock; unsigned int irq; void __iomem *reg_base; - void __iomem *phy_addr; - unsigned int enable_mask;
struct video_info *video_info; struct link_train link_train;
DP PHY now require pmu-system-controller to handle PMU register to control PHY's power isolation. Adding the same to dp-phy node.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com Cc: Jingoo Han jg1.han@samsung.com ---
Changes from V1: - none.
arch/arm/boot/dts/exynos5250.dtsi | 2 +- arch/arm/boot/dts/exynos5420.dtsi | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index 012b021..69f5eb0 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -732,7 +732,7 @@
dp_phy: video-phy@10040720 { compatible = "samsung,exynos5250-dp-video-phy"; - reg = <0x10040720 4>; + samsung,pmu-syscon = <&pmu_system_controller>; #phy-cells = <0>; };
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi index 8617a03..1353a09 100644 --- a/arch/arm/boot/dts/exynos5420.dtsi +++ b/arch/arm/boot/dts/exynos5420.dtsi @@ -503,8 +503,8 @@ };
dp_phy: video-phy@10040728 { - compatible = "samsung,exynos5250-dp-video-phy"; - reg = <0x10040728 4>; + compatible = "samsung,exynos5420-dp-video-phy"; + samsung,pmu-syscon = <&pmu_system_controller>; #phy-cells = <0>; };
On Thursday, October 30, 2014 10:24 PM, Vivek Gautam wrote:
DP PHY now require pmu-system-controller to handle PMU register to control PHY's power isolation. Adding the same to dp-phy node.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com Cc: Jingoo Han jg1.han@samsung.com
Reviewed-by: Jingoo Han jg1.han@samsung.com
Best regards, Jingoo Han
Changes from V1:
- none.
arch/arm/boot/dts/exynos5250.dtsi | 2 +- arch/arm/boot/dts/exynos5420.dtsi | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi index 012b021..69f5eb0 100644 --- a/arch/arm/boot/dts/exynos5250.dtsi +++ b/arch/arm/boot/dts/exynos5250.dtsi @@ -732,7 +732,7 @@
dp_phy: video-phy@10040720 { compatible = "samsung,exynos5250-dp-video-phy";
reg = <0x10040720 4>;
#phy-cells = <0>; };samsung,pmu-syscon = <&pmu_system_controller>;
diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi index 8617a03..1353a09 100644 --- a/arch/arm/boot/dts/exynos5420.dtsi +++ b/arch/arm/boot/dts/exynos5420.dtsi @@ -503,8 +503,8 @@ };
dp_phy: video-phy@10040728 {
compatible = "samsung,exynos5250-dp-video-phy";
reg = <0x10040728 4>;
compatible = "samsung,exynos5420-dp-video-phy";
#phy-cells = <0>; };samsung,pmu-syscon = <&pmu_system_controller>;
-- 1.7.10.4
[adding Kukjin to cc list]
Hello Vivek,
On Wed, Nov 12, 2014 at 5:21 AM, Jingoo Han jg1.han@samsung.com wrote:
On Thursday, October 30, 2014 10:24 PM, Vivek Gautam wrote:
DP PHY now require pmu-system-controller to handle PMU register to control PHY's power isolation. Adding the same to dp-phy node.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com Cc: Jingoo Han jg1.han@samsung.com
Reviewed-by: Jingoo Han jg1.han@samsung.com
Tested-by: Javier Martinez Canillas javier.martinez@collabora.co.uk
Kukjin,
This patch is -rc material and is needed to have display working in 3.18 again since commit a5ec598 ("phy: exynos-dp-video: Use syscon support to control pmu register") landed in 3.18 and broke the Exynos Display Port PHY:
exynos-dp-video-phy 10040728.video-phy: Failed to lookup PMU regmap
Best regards, Javier
Hi Javier,
On Wed, Nov 19, 2014 at 5:06 PM, Javier Martinez Canillas javier@dowhile0.org wrote:
[adding Kukjin to cc list]
Hello Vivek,
On Wed, Nov 12, 2014 at 5:21 AM, Jingoo Han jg1.han@samsung.com wrote:
On Thursday, October 30, 2014 10:24 PM, Vivek Gautam wrote:
DP PHY now require pmu-system-controller to handle PMU register to control PHY's power isolation. Adding the same to dp-phy node.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com Cc: Jingoo Han jg1.han@samsung.com
Reviewed-by: Jingoo Han jg1.han@samsung.com
Tested-by: Javier Martinez Canillas javier.martinez@collabora.co.uk
Thanks for testing.
Kukjin,
Sorry for not adding Kukjin to the list and thereby for the delay about this patch.
This patch is -rc material and is needed to have display working in 3.18 again since commit a5ec598 ("phy: exynos-dp-video: Use syscon support to control pmu register") landed in 3.18 and broke the Exynos Display Port PHY:
exynos-dp-video-phy 10040728.video-phy: Failed to lookup PMU regmap
Yes, we should pick this up and merge it since the driver patch is merged now.
Hello Vivek
On Wed, Nov 19, 2014 at 1:03 PM, Vivek Gautam gautamvivek1987@gmail.com wrote:
Tested-by: Javier Martinez Canillas javier.martinez@collabora.co.uk
Thanks for testing.
You are welcome
Kukjin,
Sorry for not adding Kukjin to the list and thereby for the delay about this patch.
No worries but I'm not sure if Kukjin is aware of this patch. I see he has been applying other patches but didn't pick $subject.
Maybe you can resend it to Kukjin just to be sure he will have it in his mailbox?
Best regards, Javier
On Sun, Nov 23, 2014 at 12:26 AM, Javier Martinez Canillas javier@dowhile0.org wrote:
Hello Vivek
On Wed, Nov 19, 2014 at 1:03 PM, Vivek Gautam gautamvivek1987@gmail.com wrote:
Tested-by: Javier Martinez Canillas javier.martinez@collabora.co.uk
Thanks for testing.
You are welcome
Kukjin,
Sorry for not adding Kukjin to the list and thereby for the delay about this patch.
No worries but I'm not sure if Kukjin is aware of this patch. I see he has been applying other patches but didn't pick $subject.
Right,
Maybe you can resend it to Kukjin just to be sure he will have it in his mailbox?
Posted a RESEND version of this patch. Thanks again for noticing.
On Thursday, October 30, 2014 10:24 PM, Vivek Gautam wrote:
Now that we have moved to generic phy based bindings, we don't need to have any code related to older dptx-phy. Nobody is using this dptx-phy anymore, so removing the same.
Right, older dptx-phy was replaced long time ago. However, it was not removed for DT compatibility. I think that now these old DT properties can be removed.
I added some comments below.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com Cc: Inki Dae inki.dae@samsung.com Cc: Jingoo Han jg1.han@samsung.com
Changes from V1:
- Reworked error handling in exynos_dp_dt_parse_phydata() as commented by Inki.
drivers/gpu/drm/exynos/exynos_dp_core.c | 67 ++++++++----------------------- drivers/gpu/drm/exynos/exynos_dp_core.h | 2 - 2 files changed, 17 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index cd50ece..206163b 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -1052,28 +1052,14 @@ static int exynos_dp_create_connector(struct exynos_drm_display *display,
static void exynos_dp_phy_init(struct exynos_dp_device *dp) {
- if (dp->phy) {
- if (dp->phy) phy_power_on(dp->phy);
- } else if (dp->phy_addr) {
u32 reg;
reg = __raw_readl(dp->phy_addr);
reg |= dp->enable_mask;
__raw_writel(reg, dp->phy_addr);
- }
}
static void exynos_dp_phy_exit(struct exynos_dp_device *dp) {
- if (dp->phy) {
- if (dp->phy) phy_power_off(dp->phy);
- } else if (dp->phy_addr) {
u32 reg;
reg = __raw_readl(dp->phy_addr);
reg &= ~(dp->enable_mask);
__raw_writel(reg, dp->phy_addr);
- }
}
static void exynos_dp_poweron(struct exynos_drm_display *display) @@ -1212,40 +1198,13 @@ static struct video_info *exynos_dp_dt_parse_pdata(struct device *dev)
static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp) {
- struct device_node *dp_phy_node = of_node_get(dp->dev->of_node);
- u32 phy_base;
- int ret = 0;
- dp_phy_node = of_find_node_by_name(dp_phy_node, "dptx-phy");
- if (!dp_phy_node) {
dp->phy = devm_phy_get(dp->dev, "dp");
return PTR_ERR_OR_ZERO(dp->phy);
- }
- if (of_property_read_u32(dp_phy_node, "reg", &phy_base)) {
dev_err(dp->dev, "failed to get reg for dptx-phy\n");
ret = -EINVAL;
goto err;
- }
- if (of_property_read_u32(dp_phy_node, "samsung,enable-mask",
&dp->enable_mask)) {
dev_err(dp->dev, "failed to get enable-mask for dptx-phy\n");
ret = -EINVAL;
goto err;
- }
- dp->phy_addr = ioremap(phy_base, SZ_4);
- if (!dp->phy_addr) {
dev_err(dp->dev, "failed to ioremap dp-phy\n");
ret = -ENOMEM;
goto err;
- dp->phy = devm_phy_get(dp->dev, "dp");
- if (IS_ERR(dp->phy)) {
dev_err(dp->dev, "no DP phy configured\n");
}return PTR_ERR(dp->phy);
-err:
- of_node_put(dp_phy_node);
- return ret;
- return 0;
}
static int exynos_dp_dt_parse_panel(struct exynos_dp_device *dp) @@ -1278,8 +1237,16 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) return PTR_ERR(dp->video_info);
ret = exynos_dp_dt_parse_phydata(dp);
In your patch, exynos_dp_dt_parse_phydata() calls only devm_phy_get(). Then, how about calling devm_phy_get() directly and removing exynos_dp_dt_parse_phydata()? It looks simpler.
Best regards, Jingoo Han
- if (ret)
return ret;
if (ret) {
/*
* phy itself is not enabled, so we can move forward
* assigning NULL to phy pointer.
*/
if (ret == -ENOSYS || ret == -ENODEV)
dp->phy = NULL;
else
return ret;
}
if (!dp->panel) { ret = exynos_dp_dt_parse_panel(dp);
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.h b/drivers/gpu/drm/exynos/exynos_dp_core.h index a1aee69..6426201 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.h +++ b/drivers/gpu/drm/exynos/exynos_dp_core.h @@ -153,8 +153,6 @@ struct exynos_dp_device { struct clk *clock; unsigned int irq; void __iomem *reg_base;
void __iomem *phy_addr;
unsigned int enable_mask;
struct video_info *video_info; struct link_train link_train;
-- 1.7.10.4
On Wed, Nov 12, 2014 at 9:38 AM, Jingoo Han jg1.han@samsung.com wrote:
On Thursday, October 30, 2014 10:24 PM, Vivek Gautam wrote:
Now that we have moved to generic phy based bindings, we don't need to have any code related to older dptx-phy. Nobody is using this dptx-phy anymore, so removing the same.
Right, older dptx-phy was replaced long time ago. However, it was not removed for DT compatibility. I think that now these old DT properties can be removed.
I added some comments below.
Thanks Jingoo for reviewing.
Signed-off-by: Vivek Gautam gautam.vivek@samsung.com Cc: Inki Dae inki.dae@samsung.com Cc: Jingoo Han jg1.han@samsung.com
Changes from V1:
- Reworked error handling in exynos_dp_dt_parse_phydata() as commented by Inki.
drivers/gpu/drm/exynos/exynos_dp_core.c | 67 ++++++++----------------------- drivers/gpu/drm/exynos/exynos_dp_core.h | 2 - 2 files changed, 17 insertions(+), 52 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index cd50ece..206163b 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -1052,28 +1052,14 @@ static int exynos_dp_create_connector(struct exynos_drm_display *display,
static void exynos_dp_phy_init(struct exynos_dp_device *dp) {
if (dp->phy) {
if (dp->phy) phy_power_on(dp->phy);
} else if (dp->phy_addr) {
u32 reg;
reg = __raw_readl(dp->phy_addr);
reg |= dp->enable_mask;
__raw_writel(reg, dp->phy_addr);
}
}
static void exynos_dp_phy_exit(struct exynos_dp_device *dp) {
if (dp->phy) {
if (dp->phy) phy_power_off(dp->phy);
} else if (dp->phy_addr) {
u32 reg;
reg = __raw_readl(dp->phy_addr);
reg &= ~(dp->enable_mask);
__raw_writel(reg, dp->phy_addr);
}
}
static void exynos_dp_poweron(struct exynos_drm_display *display) @@ -1212,40 +1198,13 @@ static struct video_info *exynos_dp_dt_parse_pdata(struct device *dev)
static int exynos_dp_dt_parse_phydata(struct exynos_dp_device *dp) {
struct device_node *dp_phy_node = of_node_get(dp->dev->of_node);
u32 phy_base;
int ret = 0;
dp_phy_node = of_find_node_by_name(dp_phy_node, "dptx-phy");
if (!dp_phy_node) {
dp->phy = devm_phy_get(dp->dev, "dp");
return PTR_ERR_OR_ZERO(dp->phy);
}
if (of_property_read_u32(dp_phy_node, "reg", &phy_base)) {
dev_err(dp->dev, "failed to get reg for dptx-phy\n");
ret = -EINVAL;
goto err;
}
if (of_property_read_u32(dp_phy_node, "samsung,enable-mask",
&dp->enable_mask)) {
dev_err(dp->dev, "failed to get enable-mask for dptx-phy\n");
ret = -EINVAL;
goto err;
}
dp->phy_addr = ioremap(phy_base, SZ_4);
if (!dp->phy_addr) {
dev_err(dp->dev, "failed to ioremap dp-phy\n");
ret = -ENOMEM;
goto err;
dp->phy = devm_phy_get(dp->dev, "dp");
if (IS_ERR(dp->phy)) {
dev_err(dp->dev, "no DP phy configured\n");
return PTR_ERR(dp->phy); }
-err:
of_node_put(dp_phy_node);
return ret;
return 0;
}
static int exynos_dp_dt_parse_panel(struct exynos_dp_device *dp) @@ -1278,8 +1237,16 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) return PTR_ERR(dp->video_info);
ret = exynos_dp_dt_parse_phydata(dp);
In your patch, exynos_dp_dt_parse_phydata() calls only devm_phy_get(). Then, how about calling devm_phy_get() directly and removing exynos_dp_dt_parse_phydata()? It looks simpler.
Right, makes sense. Will send quick rework for this. Then you can give your Reviewed-by. ;-)
[snip]
dri-devel@lists.freedesktop.org