2014-10-30 13:24:12

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH v2 1/2] drm/exynos: dp: Remove support for unused dptx-phy

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 <[email protected]>
Cc: Inki Dae <[email protected]>
Cc: Jingoo Han <[email protected]>
---

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;
--
1.7.10.4


2014-10-30 13:24:16

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH v2 2/2] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

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 <[email protected]>
Cc: Jingoo Han <[email protected]>
---

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>;
};

--
1.7.10.4

2014-11-12 04:08:16

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] drm/exynos: dp: Remove support for unused dptx-phy

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 <[email protected]>
> Cc: Inki Dae <[email protected]>
> Cc: Jingoo Han <[email protected]>
> ---
>
> 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

2014-11-12 04:20:30

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

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 <[email protected]>
> Cc: Jingoo Han <[email protected]>

Reviewed-by: Jingoo Han <[email protected]>

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>;
> + 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>;
> };
>
> --
> 1.7.10.4

2014-11-12 08:25:53

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] drm/exynos: dp: Remove support for unused dptx-phy

On Wed, Nov 12, 2014 at 9:38 AM, Jingoo Han <[email protected]> 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 <[email protected]>
>> Cc: Inki Dae <[email protected]>
>> Cc: Jingoo Han <[email protected]>
>> ---
>>
>> 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]



--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

On Wed, Nov 12, 2014 at 9:38 AM, Jingoo Han <[email protected]> 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.
>
>>
>> Signed-off-by: Vivek Gautam <[email protected]>
>> Cc: Inki Dae <[email protected]>
>> Cc: Jingoo Han <[email protected]>
>> ---
>>
>> 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

2014-11-12 09:28:07

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH v3 1/2] drm/exynos: dp: Remove support for unused dptx-phy

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 <[email protected]>
Cc: Inki Dae <[email protected]>
Cc: Jingoo Han <[email protected]>
---

Changes from V2:
- Moved devm_phy_get() call out of exynos_dp_dt_parse_phydata() to
exynos_dp_bind() function and,
removed exynos_dp_dt_parse_phydata() function, since it was only
getting the PHY.

Changes from V1:
- Reworked error handling in exynos_dp_dt_parse_phydata() as commented
by Inki.

drivers/gpu/drm/exynos/exynos_dp_core.c | 74 +++++++------------------------
drivers/gpu/drm/exynos/exynos_dp_core.h | 2 -
2 files changed, 17 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
index cd50ece..dbe9add 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)
@@ -1210,44 +1196,6 @@ static struct video_info *exynos_dp_dt_parse_pdata(struct device *dev)
return dp_video_config;
}

-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;
- }
-
-err:
- of_node_put(dp_phy_node);
-
- return ret;
-}
-
static int exynos_dp_dt_parse_panel(struct exynos_dp_device *dp)
{
int ret;
@@ -1277,9 +1225,21 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
if (IS_ERR(dp->video_info))
return PTR_ERR(dp->video_info);

- ret = exynos_dp_dt_parse_phydata(dp);
- if (ret)
- return ret;
+ dp->phy = devm_phy_get(dp->dev, "dp");
+ if (IS_ERR(dp->phy)) {
+ dev_err(dp->dev, "no DP phy configured\n");
+ ret = PTR_ERR(dp->phy);
+ 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

2014-11-12 09:40:29

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] drm/exynos: dp: Remove support for unused dptx-phy

On Wednesday, November 12, 2014 6:28 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.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> Cc: Inki Dae <[email protected]>
> Cc: Jingoo Han <[email protected]>

Acked-by: Jingoo Han <[email protected]>

Best regards,
Jingoo Han

> ---
>
> Changes from V2:
> - Moved devm_phy_get() call out of exynos_dp_dt_parse_phydata() to
> exynos_dp_bind() function and,
> removed exynos_dp_dt_parse_phydata() function, since it was only
> getting the PHY.
>
> Changes from V1:
> - Reworked error handling in exynos_dp_dt_parse_phydata() as commented
> by Inki.
>
> drivers/gpu/drm/exynos/exynos_dp_core.c | 74 +++++++------------------------
> drivers/gpu/drm/exynos/exynos_dp_core.h | 2 -
> 2 files changed, 17 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> index cd50ece..dbe9add 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)
> @@ -1210,44 +1196,6 @@ static struct video_info *exynos_dp_dt_parse_pdata(struct device *dev)
> return dp_video_config;
> }
>
> -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;
> - }
> -
> -err:
> - of_node_put(dp_phy_node);
> -
> - return ret;
> -}
> -
> static int exynos_dp_dt_parse_panel(struct exynos_dp_device *dp)
> {
> int ret;
> @@ -1277,9 +1225,21 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
> if (IS_ERR(dp->video_info))
> return PTR_ERR(dp->video_info);
>
> - ret = exynos_dp_dt_parse_phydata(dp);
> - if (ret)
> - return ret;
> + dp->phy = devm_phy_get(dp->dev, "dp");
> + if (IS_ERR(dp->phy)) {
> + dev_err(dp->dev, "no DP phy configured\n");
> + ret = PTR_ERR(dp->phy);
> + 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

2014-11-19 11:36:15

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

[adding Kukjin to cc list]

Hello Vivek,

On Wed, Nov 12, 2014 at 5:21 AM, Jingoo Han <[email protected]> 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 <[email protected]>
>> Cc: Jingoo Han <[email protected]>
>
> Reviewed-by: Jingoo Han <[email protected]>
>

Tested-by: Javier Martinez Canillas <[email protected]>

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

2014-11-19 12:04:00

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

Hi Javier,


On Wed, Nov 19, 2014 at 5:06 PM, Javier Martinez Canillas
<[email protected]> wrote:
> [adding Kukjin to cc list]
>
> Hello Vivek,
>
> On Wed, Nov 12, 2014 at 5:21 AM, Jingoo Han <[email protected]> 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 <[email protected]>
>>> Cc: Jingoo Han <[email protected]>
>>
>> Reviewed-by: Jingoo Han <[email protected]>
>>
>
> Tested-by: Javier Martinez Canillas <[email protected]>

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.



--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

2014-11-22 18:56:20

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

Hello Vivek

On Wed, Nov 19, 2014 at 1:03 PM, Vivek Gautam <[email protected]> wrote:
>>
>> Tested-by: Javier Martinez Canillas <[email protected]>
>
> 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

2014-11-24 09:28:07

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] arm: dts: Exynos5: Use pmu_system_controller phandle for dp phy

On Sun, Nov 23, 2014 at 12:26 AM, Javier Martinez Canillas
<[email protected]> wrote:
> Hello Vivek
>
> On Wed, Nov 19, 2014 at 1:03 PM, Vivek Gautam <[email protected]> wrote:
>>>
>>> Tested-by: Javier Martinez Canillas <[email protected]>
>>
>> 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.



--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India