Skip to contentSkip to conversation

phy: Add driver for Exynos DP PHY

PATCH Superseded Jingoo Han 2 participants 3 messages Jul 1, 2013 – Jul 2, 2013

Subsystems (5)
Device tree bindings Media (V4L/DVB) Framebuffer layer Arm Samsung Exynos +1 more subsystems

v3 of 8 Superseded Latest is v8 →

lore
Share & tools
lore

Jingoo Han

author
#0 lore raw

PATCH v3 2/3 phy: Add driver for Exynos DP PHY

Add a PHY provider driver for the Samsung Exynos SoC DP PHY.

Signed-off-by Jingoo Han <[email protected]>
Cc Sylwester Nawrocki <[email protected]>
Acked-by Felipe Balbi <[email protected]>
---
 .../devicetree/bindings/phy/samsung-phy.txt        |    8 ++
 drivers/phy/Kconfig                                |    5 +
 drivers/phy/Makefile                               |    1 +
 drivers/phy/phy-exynos-dp-video.c                  |  118 ++++++++++++++++++++
 4 files changed, 132 insertions(+)
 create mode 100644 drivers/phy/phy-exynos-dp-video.c
Documentation/devicetree/bindings/phy/samsung-phy.txt +8 -0
index 5ff208c..3fb656a 100644
@@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
   1 - MIPI DSIM 0,
   2 - MIPI CSIS 1,
   3 - MIPI DSIM 1.
+
+Samsung EXYNOS SoC series DP PHY
+-------------------------------------------------
+
+Required properties:
+- compatible : should be "samsung,exynos5250-dp-video-phy";
+- reg : offset and length of the DP PHY register set;
+- #phy-cells : from the generic phy bindings, must be 0;
\ No newline at end of file
drivers/phy/Kconfig +5 -0
index 6f446d0..760f55a 100644
@@ -19,4 +19,9 @@ config PHY_EXYNOS_MIPI_VIDEO
 	help
 	  Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
 	  S5P and EXYNOS SoCs.
+
+config PHY_EXYNOS_DP_VIDEO
+	tristate "EXYNOS SoC series DP PHY driver"
+	help
+	  Support for DP PHY found on Samsung EXYNOS SoCs.
 endif
drivers/phy/Makefile +1 -0
index 71d8841..0fd1340 100644
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
 obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
+obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
drivers/phy/phy-exynos-dp-video.c new file +118 -0

118 lines not on this page · raw message

Signature · 1 line
1.7.10.4
#1 lore raw

PATCH v3 2/3 phy: Add driver for Exynos DP PHY

Quoted 18 lines · Jingoo Han

Add a PHY provider driver for the Samsung Exynos SoC DP PHY.

Signed-off-by Jingoo Han<[email protected]>
Cc Sylwester Nawrocki<[email protected]>
Acked-by Felipe Balbi<[email protected]>
---
  .../devicetree/bindings/phy/samsung-phy.txt        |    8 ++
  drivers/phy/Kconfig                                |    5 +
  drivers/phy/Makefile                               |    1 +
  drivers/phy/phy-exynos-dp-video.c                  |  118 ++++++++++++++++++++
  4 files changed, 132 insertions(+)

create mode 100644 drivers/phy/phy-exynos-dp-video.c

Documentation/devicetree/bindings/phy/samsung-phy.txt +0 -0
index 5ff208c..3fb656a 100644

Documentation/devicetree/bindings/phy/samsung-phy.txt the PHY specifier identifies the PHY and its meaning is as follows: · line 16

Quoted 5 lines
Documentation/devicetree/bindings/phy/samsung-phy.txt +2 -0
@@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
    1 - MIPI DSIM 0,
    2 - MIPI CSIS 1,
    3 - MIPI DSIM 1.
+
+Samsung EXYNOS SoC series DP PHY

I would make it "Samsung EXYNOS SoC series Display Port PHY"

Documentation/devicetree/bindings/phy/samsung-phy.txt the PHY specifier identifies the PHY and its meaning is as follows: · line 22

Quoted 6 lines
Documentation/devicetree/bindings/phy/samsung-phy.txt +6 -0
+-------------------------------------------------
+
+Required properties:
+- compatible : should be "samsung,exynos5250-dp-video-phy";
+- reg : offset and length of the DP PHY register set;
+- #phy-cells : from the generic phy bindings, must be 0;

s/phy/PHY ?

Quoted 1 line

\ No newline at end of file

Missing new line character.

drivers/phy/Kconfig config PHY_EXYNOS_MIPI_VIDEO · line 24

Quoted 11 lines
drivers/phy/Kconfig +3 -0
index 6f446d0..760f55a 100644
@@ -19,4 +19,9 @@ config PHY_EXYNOS_MIPI_VIDEO
  	help
  	  Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
  	  S5P and EXYNOS SoCs.
+
+config PHY_EXYNOS_DP_VIDEO
+	tristate "EXYNOS SoC series DP PHY driver"

"EXYNOS SoC series Display Port PHY driver" ?

drivers/phy/Kconfig config PHY_EXYNOS_MIPI_VIDEO · line 26

Quoted 2 lines
drivers/phy/Kconfig +2 -0
+	help
+	  Support for DP PHY found on Samsung EXYNOS SoCs.

s/DP/Display Port ?

Quoted 16 lines
drivers/phy/Kconfig +0 -0
  endif
drivers/phy/Makefile +1 -0
index 71d8841..0fd1340 100644
@@ -4,3 +4,4 @@
 
  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
+obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
drivers/phy/phy-exynos-dp-video.c new file +0 -0
new file mode 100644
index 0000000..75e1d11

drivers/phy/phy-exynos-dp-video.c line 2

Quoted 2 lines
drivers/phy/phy-exynos-dp-video.c new file +2 -0
@@ -0,0 +1,118 @@
+/*
+ * Samsung EXYNOS SoC series DP PHY driver

s/DP/Display Port ?

Quoted 17 lines
drivers/phy/phy-exynos-dp-video.c new file +17 -0
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Jingoo Han<[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include<linux/io.h>
+#include<linux/kernel.h>
+#include<linux/module.h>
+#include<linux/of.h>
+#include<linux/of_address.h>
+#include<linux/phy/phy.h>
+#include<linux/platform_device.h>
+

drivers/phy/phy-exynos-dp-video.c line 29

Quoted 10 lines
drivers/phy/phy-exynos-dp-video.c new file +10 -0
+/* DPTX_PHY_CONTROL register */
+#define EXYNOS_DPTX_PHY_ENABLE		(1<<  0)
+
+struct exynos_dp_video_phy {
+	void __iomem *regs;
+};
+
+static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
+{
+	void __iomem *addr;

How about just dropping this local variable ?

Quoted 43 lines
drivers/phy/phy-exynos-dp-video.c new file +43 -0
+	u32 reg;
+
+	addr = state->regs;
+
+	reg = readl(addr);
+	if (on)
+		reg |= EXYNOS_DPTX_PHY_ENABLE;
+	else
+		reg&= ~EXYNOS_DPTX_PHY_ENABLE;
+	writel(reg, addr);
+
+	return 0;
+}
+
+static int exynos_dp_video_phy_power_on(struct phy *phy)
+{
+	struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+	return __set_phy_state(state, 1);
+}
+
+static int exynos_dp_video_phy_power_off(struct phy *phy)
+{
+	struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+	return __set_phy_state(state, 0);
+}
+
+static struct phy_ops exynos_dp_video_phy_ops = {
+	.power_on	= exynos_dp_video_phy_power_on,
+	.power_off	= exynos_dp_video_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static int exynos_dp_video_phy_probe(struct platform_device *pdev)
+{
+	struct exynos_dp_video_phy *state;
+	struct device *dev =&pdev->dev;
+	struct resource *res;
+	struct phy_provider *phy_provider;
+	struct phy *phy;
+
+	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);

drivers/phy/phy-exynos-dp-video.c line 82

Quoted 10 lines
drivers/phy/phy-exynos-dp-video.c new file +10 -0
+	if (!state)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	state->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(state->regs))
+		return PTR_ERR(state->regs);
+
+	dev_set_drvdata(dev, state);

I can't see any corresponding dev_get_drvdata(), it should be safe to remove this assignment.

drivers/phy/phy-exynos-dp-video.c

Quoted 5 lines
drivers/phy/phy-exynos-dp-video.c new file +5 -0
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	phy = devm_phy_create(dev, 0,&exynos_dp_video_phy_ops, "dp");

The label could be NULL, since there is no need to support non-dt config.

drivers/phy/phy-exynos-dp-video.c line 98

Quoted 10 lines
drivers/phy/phy-exynos-dp-video.c new file +10 -0
+	if (IS_ERR(phy)) {
+		dev_err(dev, "failed to create DP PHY\n");
+		return PTR_ERR(phy);
+	}
+	phy_set_drvdata(phy, state);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF

You don't use of_match_ptr() macro to assign of_match_table to the driver structure below. This means build with CONFIG_OF disabled is broken now. I would just remove that #ifdef and would make the driver depends on CONFIG_OF.

Quoted 10 lines
drivers/phy/phy-exynos-dp-video.c new file +10 -0
+static const struct of_device_id exynos_dp_video_phy_of_match[] = {
+	{ .compatible = "samsung,exynos5250-dp-video-phy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
+#endif
+
+static struct platform_driver exynos_dp_video_phy_driver = {
+	.probe	= exynos_dp_video_phy_probe,
+	.driver = {

drivers/phy/phy-exynos-dp-video.c line 118

Quoted 10 lines
drivers/phy/phy-exynos-dp-video.c new file +10 -0
+		.name	= "exynos-dp-video-phy",
+		.owner	= THIS_MODULE,
+		.of_match_table	= exynos_dp_video_phy_of_match,
+	}
+};
+module_platform_driver(exynos_dp_video_phy_driver);
+
+MODULE_AUTHOR("Jingoo Han<[email protected]>");
+MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
+MODULE_LICENSE("GPL v2");

Thanks,
Sylwester

author
#2 lore raw

PATCH v3 2/3 phy: Add driver for Exynos DP PHY

Quoted 16 lines · Sylwester Nawrocki
Quoted 11 lines · Jingoo Han

Add a PHY provider driver for the Samsung Exynos SoC DP PHY.

Signed-off-by Jingoo Han<[email protected]>
Cc Sylwester Nawrocki<[email protected]>
Acked-by Felipe Balbi<[email protected]>
---
  .../devicetree/bindings/phy/samsung-phy.txt        |    8 ++
  drivers/phy/Kconfig                                |    5 +
  drivers/phy/Makefile                               |    1 +
  drivers/phy/phy-exynos-dp-video.c                  |  118 ++++++++++++++++++++
  4 files changed, 132 insertions(+)

Review

Quoted 3 lines

create mode 100644 drivers/phy/phy-exynos-dp-video.c

(quoted diff) +0 -0

b/Documentation/devicetree/bindings/phy/samsung-phy.txt

Quoted 11 lines

Documentation/devicetree/bindings/phy/samsung-phy.txt the PHY specifier identifies the PHY and its meaning is as follows: · line 16

Quoted 9 lines

index 5ff208c..3fb656a 100644

Documentation/devicetree/bindings/phy/samsung-phy.txt +2 -0
@@ -12,3 +12,11 @@ the PHY specifier identifies the PHY and its meaning is as follows:
    1 - MIPI DSIM 0,
    2 - MIPI CSIS 1,
    3 - MIPI DSIM 1.
+
+Samsung EXYNOS SoC series DP PHY

I would make it "Samsung EXYNOS SoC series Display Port PHY"

OK.

Quoted 9 lines

Documentation/devicetree/bindings/phy/samsung-phy.txt the PHY specifier identifies the PHY and its meaning is as follows: · line 22

Quoted 7 lines
Documentation/devicetree/bindings/phy/samsung-phy.txt +6 -0
+-------------------------------------------------
+
+Required properties:
+- compatible : should be "samsung,exynos5250-dp-video-phy";
+- reg : offset and length of the DP PHY register set;
+- #phy-cells : from the generic phy bindings, must be 0;

s/phy/PHY ?

OK.

Quoted 4 lines
Quoted 2 lines

\ No newline at end of file

Missing new line character.

OK.

Quoted 15 lines

drivers/phy/Kconfig config PHY_EXYNOS_MIPI_VIDEO · line 24

Quoted 12 lines
drivers/phy/Kconfig +3 -0
index 6f446d0..760f55a 100644
@@ -19,4 +19,9 @@ config PHY_EXYNOS_MIPI_VIDEO
  	help
  	  Support for MIPI CSI-2 and MIPI DSI DPHY found on Samsung
  	  S5P and EXYNOS SoCs.
+
+config PHY_EXYNOS_DP_VIDEO
+	tristate "EXYNOS SoC series DP PHY driver"

"EXYNOS SoC series Display Port PHY driver" ?

Quoted 4 lines

drivers/phy/Kconfig config PHY_EXYNOS_MIPI_VIDEO · line 26

Quoted 2 lines
drivers/phy/Kconfig +2 -0
+	help
+	  Support for DP PHY found on Samsung EXYNOS SoCs.

s/DP/Display Port ?

OK.

Quoted 8 lines
Quoted 8 lines
drivers/phy/Kconfig +0 -0
  endif
drivers/phy/Makefile +0 -0
index 71d8841..0fd1340 100644
@@ -4,3 +4,4 @@
Quoted 13 lines

drivers/phy/phy-exynos-dp-video.c line 2

Quoted 11 lines
drivers/phy/Makefile +1 -0
  obj-$(CONFIG_GENERIC_PHY)		+= phy-core.o
  obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)	+= phy-exynos-mipi-video.o
+obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO)	+= phy-exynos-dp-video.o
drivers/phy/phy-exynos-dp-video.c new file +2 -0
new file mode 100644
index 0000000..75e1d11
@@ -0,0 +1,118 @@
+/*
+ * Samsung EXYNOS SoC series DP PHY driver

s/DP/Display Port ?

OK.

Quoted 22 lines
Quoted 22 lines
drivers/phy/phy-exynos-dp-video.c new file +21 -0
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Jingoo Han<[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include<linux/io.h>
+#include<linux/kernel.h>
+#include<linux/module.h>
+#include<linux/of.h>
+#include<linux/of_address.h>
+#include<linux/phy/phy.h>
+#include<linux/platform_device.h>
+
+/* DPTX_PHY_CONTROL register */
+#define EXYNOS_DPTX_PHY_ENABLE		(1<<  0)
+
+struct exynos_dp_video_phy {
Quoted 8 lines

drivers/phy/phy-exynos-dp-video.c line 29

Quoted 6 lines
drivers/phy/phy-exynos-dp-video.c new file +6 -0
+	void __iomem *regs;
+};
+
+static int __set_phy_state(struct exynos_dp_video_phy *state, unsigned int on)
+{
+	void __iomem *addr;

How about just dropping this local variable ?

OK.
I will drop this local variable.

Quoted 49 lines
Quoted 49 lines
drivers/phy/phy-exynos-dp-video.c new file +48 -0
+	u32 reg;
+
+	addr = state->regs;
+
+	reg = readl(addr);
+	if (on)
+		reg |= EXYNOS_DPTX_PHY_ENABLE;
+	else
+		reg&= ~EXYNOS_DPTX_PHY_ENABLE;
+	writel(reg, addr);
+
+	return 0;
+}
+
+static int exynos_dp_video_phy_power_on(struct phy *phy)
+{
+	struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+	return __set_phy_state(state, 1);
+}
+
+static int exynos_dp_video_phy_power_off(struct phy *phy)
+{
+	struct exynos_dp_video_phy *state = phy_get_drvdata(phy);
+
+	return __set_phy_state(state, 0);
+}
+
+static struct phy_ops exynos_dp_video_phy_ops = {
+	.power_on	= exynos_dp_video_phy_power_on,
+	.power_off	= exynos_dp_video_phy_power_off,
+	.owner		= THIS_MODULE,
+};
+
+static int exynos_dp_video_phy_probe(struct platform_device *pdev)
+{
+	struct exynos_dp_video_phy *state;
+	struct device *dev =&pdev->dev;
+	struct resource *res;
+	struct phy_provider *phy_provider;
+	struct phy *phy;
+
+	state = devm_kzalloc(dev, sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
Quoted 8 lines

drivers/phy/phy-exynos-dp-video.c line 82

Quoted 5 lines
drivers/phy/phy-exynos-dp-video.c new file +5 -0
+	state->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(state->regs))
+		return PTR_ERR(state->regs);
+
+	dev_set_drvdata(dev, state);

I can't see any corresponding dev_get_drvdata(), it should be safe to remove this assignment.

OK.
I will remove 'dev_set_drvdata(dev, state)'.

Quoted 8 lines

drivers/phy/phy-exynos-dp-video.c

Quoted 6 lines
drivers/phy/phy-exynos-dp-video.c new file +5 -0
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	if (IS_ERR(phy_provider))
+		return PTR_ERR(phy_provider);
+
+	phy = devm_phy_create(dev, 0,&exynos_dp_video_phy_ops, "dp");

The label could be NULL, since there is no need to support non-dt config.

OK.
I will replace "dp" with NULL.

Quoted 16 lines

drivers/phy/phy-exynos-dp-video.c line 98

Quoted 11 lines
drivers/phy/phy-exynos-dp-video.c new file +10 -0
+	if (IS_ERR(phy)) {
+		dev_err(dev, "failed to create DP PHY\n");
+		return PTR_ERR(phy);
+	}
+	phy_set_drvdata(phy, state);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF

You don't use of_match_ptr() macro to assign of_match_table to the driver structure below. This means build with CONFIG_OF disabled is broken now. I would just remove that #ifdef and would make the driver depends on CONFIG_OF.

OK.
I will add 'depends on OF', then remove '#ifdef CONFIG_OF'.

Thank you for your detailed comment. :)

Best regards,
Jingoo Han

Quoted 24 lines
Quoted 11 lines
drivers/phy/phy-exynos-dp-video.c new file +10 -0
+static const struct of_device_id exynos_dp_video_phy_of_match[] = {
+	{ .compatible = "samsung,exynos5250-dp-video-phy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, exynos_dp_video_phy_of_match);
+#endif
+
+static struct platform_driver exynos_dp_video_phy_driver = {
+	.probe	= exynos_dp_video_phy_probe,
+	.driver = {

drivers/phy/phy-exynos-dp-video.c line 118

Quoted 10 lines
drivers/phy/phy-exynos-dp-video.c new file +10 -0
+		.name	= "exynos-dp-video-phy",
+		.owner	= THIS_MODULE,
+		.of_match_table	= exynos_dp_video_phy_of_match,
+	}
+};
+module_platform_driver(exynos_dp_video_phy_driver);
+
+MODULE_AUTHOR("Jingoo Han<[email protected]>");
+MODULE_DESCRIPTION("Samsung EXYNOS SoC DP PHY driver");
+MODULE_LICENSE("GPL v2");

Thanks,
Sylwester